[Lldb-commits] [lldb] a59444a - [LLDB] [Windows] Initial support for ARM register contexts

[Martin Storsjo via lldb-commits]

Author: Martin Storsjo
Date: 2019-10-21T08:02:34Z
New Revision: a59444a35608988e727fe3761e34f1fad6097617

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

LOG: [LLDB] [Windows] Initial support for ARM register contexts

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

llvm-svn: 375392

Added: 

lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp

lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.h

lldb/source/Plugins/Process/Windows/Common/arm/RegisterContextWindows_arm.cpp
lldb/source/Plugins/Process/Windows/Common/arm/RegisterContextWindows_arm.h
lldb/test/Shell/Register/Inputs/arm-fp-read.cpp
lldb/test/Shell/Register/Inputs/arm-gp-read.cpp
lldb/test/Shell/Register/arm-fp-read.test
lldb/test/Shell/Register/arm-gp-read.test

Modified: 
lldb/source/Plugins/Process/Windows/Common/CMakeLists.txt
lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
llvm/utils/lit/lit/llvm/config.py

Removed: 




diff  --git a/lldb/source/Plugins/Process/Windows/Common/CMakeLists.txt 
b/lldb/source/Plugins/Process/Windows/Common/CMakeLists.txt
index d0d3fcbee6f4..876bc8cab966 100644
--- a/lldb/source/Plugins/Process/Windows/Common/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/Windows/Common/CMakeLists.txt
@@ -4,6 +4,7 @@ add_lldb_library(lldbPluginProcessWindowsCommon PLUGIN
   LocalDebugDelegate.cpp
   NativeProcessWindows.cpp
   NativeRegisterContextWindows.cpp
+  NativeRegisterContextWindows_arm.cpp
   NativeRegisterContextWindows_arm64.cpp
   NativeRegisterContextWindows_i386.cpp
   NativeRegisterContextWindows_WoW64.cpp
@@ -14,10 +15,10 @@ add_lldb_library(lldbPluginProcessWindowsCommon PLUGIN
   ProcessWindowsLog.cpp
   RegisterContextWindows.cpp
   TargetThreadWindows.cpp
+  arm/RegisterContextWindows_arm.cpp
   arm64/RegisterContextWindows_arm64.cpp
   x64/RegisterContextWindows_x64.cpp
   x86/RegisterContextWindows_x86.cpp
-  # TODO add support for ARM (NT)
 
   LINK_LIBS
 lldbCore

diff  --git 
a/lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
 
b/lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
new file mode 100644
index ..d25b08f7ecba
--- /dev/null
+++ 
b/lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
@@ -0,0 +1,644 @@
+//===-- NativeRegisterContextWindows_arm.cpp *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#if defined(__arm__) || defined(_M_ARM)
+
+#include "NativeRegisterContextWindows_arm.h"
+#include "NativeThreadWindows.h"
+#include "Plugins/Process/Utility/RegisterInfoPOSIX_arm.h"
+#include "ProcessWindowsLog.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Host/HostThread.h"
+#include "lldb/Host/windows/HostThreadWindows.h"
+#include "lldb/Host/windows/windows.h"
+
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/RegisterValue.h"
+#include "llvm/ADT/STLExtras.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+#define REG_CONTEXT_SIZE sizeof(::CONTEXT)
+
+namespace {
+static const uint32_t g_gpr_regnums_arm[] = {
+gpr_r0_arm, gpr_r1_arm,   gpr_r2_arm,  gpr_r3_arm, gpr_r4_arm,
+gpr_r5_arm, gpr_r6_arm,   gpr_r7_arm,  gpr_r8_arm, gpr_r9_arm,
+gpr_r10_arm,gpr_r11_arm,  gpr_r12_arm, gpr_sp_arm, gpr_lr_arm,
+gpr_pc_arm, gpr_cpsr_arm,
+LLDB_INVALID_REGNUM // Register set must be terminated with this flag
+};
+static_assert(((sizeof g_gpr_regnums_arm / sizeof g_gpr_regnums_arm[0]) - 1) ==
+  k_num_gpr_registers_arm,
+  "g_gpr_regnums_arm has wrong number of register infos");
+
+static const uint32_t g_fpr_regnums_arm[] = {
+fpu_s0_arm, fpu_s1_arm,  fpu_s2_arm,  fpu_s3_arm,  fpu_s4_arm,
+fpu_s5_arm, fpu_s6_arm,  fpu_s7_arm,  fpu_s8_arm,  fpu_s9_arm,
+fpu_s10_arm,fpu_s11_arm, fpu_s12_arm, fpu_s13_arm, fpu_s14_arm,
+fpu_s15_arm,fpu_s16_arm, fpu_s17_arm, fpu_s18_arm, fpu_s19_arm,
+fpu_s20_arm,fpu_s21_arm, fpu_s22_arm, fpu_s23_arm, fpu_s24_arm,
+fpu_s25_arm,fpu_s26_arm, fpu_s27_arm, fpu_s28_arm, fpu_s29_arm,
+fpu_s30_arm,fpu_s31_arm,
+
+fpu_d0_arm, fpu_d1_arm,  fpu_d2_arm,  fpu_d3_arm,  fpu_d4_arm,
+fpu_d5_arm, fpu_d6_arm,  fpu_d7_arm,  fpu_d8_arm,  fpu_d9_arm,
+fpu_d10_arm,fpu_d11_arm, fpu_d12_arm, fpu_d13_arm, fpu_d14_arm,
+fpu_d15_arm,fpu_d16_arm, fpu_d1

[Lldb-commits] [PATCH] D69226: [LLDB] [Windows] Initial support for ARM register contexts

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa59444a35608: [LLDB] [Windows] Initial support for ARM 
register contexts (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69226

Files:
  lldb/source/Plugins/Process/Windows/Common/CMakeLists.txt
  
lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
  lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.h
  lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/arm/RegisterContextWindows_arm.cpp
  lldb/source/Plugins/Process/Windows/Common/arm/RegisterContextWindows_arm.h
  lldb/test/Shell/Register/Inputs/arm-fp-read.cpp
  lldb/test/Shell/Register/Inputs/arm-gp-read.cpp
  lldb/test/Shell/Register/arm-fp-read.test
  lldb/test/Shell/Register/arm-gp-read.test
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -99,6 +99,8 @@
 features.add('target-x86_64')
 elif re.match(r'^aarch64.*', target_triple):
 features.add('target-aarch64')
+elif re.match(r'^arm.*', target_triple):
+features.add('target-arm')
 
 use_gmalloc = lit_config.params.get('use_gmalloc', None)
 if lit.util.pythonize_bool(use_gmalloc):
Index: lldb/test/Shell/Register/arm-gp-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/arm-gp-read.test
@@ -0,0 +1,19 @@
+# REQUIRES: native && target-arm
+# RUN: %clangxx -fomit-frame-pointer %p/Inputs/arm-gp-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+
+register read --all
+# CHECK-DAG: r0 = 0x00010203
+# CHECK-DAG: r1 = 0x10111213
+# CHECK-DAG: r2 = 0x20212223
+# CHECK-DAG: r3 = 0x30313233
+# CHECK-DAG: r4 = 0x40414243
+# CHECK-DAG: r5 = 0x50515253
+# CHECK-DAG: r6 = 0x60616263
+# CHECK-DAG: r7 = 0x70717273
+
+# CHECK-DAG: q0 = {0x08 0x09 0x0a 0x0b 0x0c 0x0d 0x0e 0x0f 0x10 0x11 0x12 0x13 0x14 0x15 0x16 0x17}
+# CHECK-DAG: q1 = {0x09 0x0a 0x0b 0x0c 0x0d 0x0e 0x0f 0x10 0x11 0x12 0x13 0x14 0x15 0x16 0x17 0x18}
+# CHECK-DAG: q2 = {0x0a 0x0b 0x0c 0x0d 0x0e 0x0f 0x10 0x11 0x12 0x13 0x14 0x15 0x16 0x17 0x18 0x19}
+# CHECK-DAG: q3 = {0x0b 0x0c 0x0d 0x0e 0x0f 0x10 0x11 0x12 0x13 0x14 0x15 0x16 0x17 0x18 0x19 0x1a}
Index: lldb/test/Shell/Register/arm-fp-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/arm-fp-read.test
@@ -0,0 +1,21 @@
+# REQUIRES: native && target-arm
+# RUN: %clangxx -fomit-frame-pointer %p/Inputs/arm-fp-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+
+register read d0
+register read d1
+register read d2
+register read d3
+register read s8
+register read s9
+register read s10
+register read s11
+# CHECK-DAG: d0 = 0.5
+# CHECK-DAG: d1 = 1.5
+# CHECK-DAG: d2 = 2.5
+# CHECK-DAG: d3 = 3.5
+# CHECK-DAG: s8 = 4.5
+# CHECK-DAG: s9 = 5.5
+# CHECK-DAG: s10 = 6.5
+# CHECK-DAG: s11 = 7.5
Index: lldb/test/Shell/Register/Inputs/arm-gp-read.cpp
===
--- /dev/null
+++ lldb/test/Shell/Register/Inputs/arm-gp-read.cpp
@@ -0,0 +1,44 @@
+#include 
+
+struct alignas(16) vec_t {
+  uint64_t a, b;
+};
+
+int main() {
+  constexpr uint32_t gprs[] = {
+0x00010203,
+0x10111213,
+0x20212223,
+0x30313233,
+0x40414243,
+0x50515253,
+0x60616263,
+0x70717273,
+  };
+
+  constexpr vec_t vecs[] = {
+{ 0x0F0E0D0C0B0A0908, 0x1716151413121110, },
+{ 0x100F0E0D0C0B0A09, 0x1817161514131211, },
+{ 0x11100F0E0D0C0B0A, 0x1918171615141312, },
+{ 0x1211100F0E0D0C0B, 0x1A19181716151413, },
+  };
+  const vec_t *vec_ptr = vecs;
+
+  asm volatile(
+"ldrd r0,  r1,  [%1]\n\t"
+"ldrd r2,  r3,  [%1, #8]\n\t"
+"ldrd r4,  r5,  [%1, #16]\n\t"
+"ldrd r6,  r7,  [%1, #24]\n\t"
+"\n\t"
+"vld1.64  {q0, q1}, [%0]!\n\t"
+"vld1.64  {q2, q3}, [%0]!\n\t"
+"\n\t"
+"bkpt #0\n\t"
+: "+r"(vec_ptr)
+: "r"(gprs)
+: "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
+  "q0", "q1", "q2", "q3"
+  );
+
+  return 0;
+}
Index: lldb/test/Shell/Register/Inputs/arm-fp-read.cpp
===
--- /dev/null
+++ lldb/test/Shell/Register/Inputs/arm-fp-read.cpp
@@ -0,0 +1,19 @@
+int main() {
+  asm volatile(
+"vmov.f64 d0,  #0.5\n\t"
+"vmov.f64 d1,  #1.5\n\t"
+"vmov.f64 d2,  #2.5\n\t"
+"vmov.f64 d3,  #3.5\n\t"
+"vmov.f32 s8,  #4.5\n\t"
+"vmov.f32 s9,  #5.5\n\t"
+"vmov.f32 s10, #6.5\n\t"
+"vmov.f32 s11, #7.5\n\t"
+"\n\t"
+"bkpt #0\n\t"
+:
+:
+: 

[Lldb-commits] [PATCH] D69254: [lldb] drop .symtab removal in minidebuginfo tests

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk planned changes to this revision.
kwk added a comment.

Still tests are running locally...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69254



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

[Lldb-commits] [PATCH] D69254: [lldb] drop .symtab removal in minidebuginfo tests

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added subscribers: lldb-commits, MaskRay, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a reviewer: alexshap.
Herald added a project: LLDB.
kwk planned changes to this revision.
kwk added a comment.

Still tests are running locally...


After D69041 , we no longer have to manually 
remove the .symtab section
once yaml2obj was run.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69254

Files:
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml


Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
@@ -5,11 +5,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj 2>&1 | FileCheck %s
 
 # CHECK: warning: (x86_64) {{.*}}.obj No LZMA support found for reading 
.gnu_debugdata section
Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
@@ -2,11 +2,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj | FileCheck %s
 
 # CHECK: [ 0] 1 X Code 0x004005b0 0x000f 0x0012 
multiplyByFour
Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
@@ -5,11 +5,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj 2>&1 | FileCheck %s
 
 # CHECK: warning: (x86_64) {{.*}}.obj An error occurred while decompression 
the section .gnu_debugdata: lzma_stream_buffer_decode()=lzma error: 
LZMA_DATA_ERROR


Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
@@ -5,11 +5,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj 2>&1 | FileCheck %s
 
 # CHECK: warning: (x86_64) {{.*}}.obj No LZMA support found for reading .gnu_debugdata section
Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
@@ -2,11 +2,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj | FileCheck %s
 
 # CHECK: [ 0] 1 X Code 0x004005b0 0x000f 0x0012 multiplyByFour
Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
@@ -5,11 +5,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj 2>&1 | FileCheck %s
 
 # CHECK: warning: (x86_64) {{.*}}.obj An error occurred while decompression the section .gnu_debugdata: lzma_stream_buffer_decode()=lzma error: LZMA_DATA_ERROR
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69254: [lldb] drop .symtab removal in minidebuginfo tests

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk requested review of this revision.
kwk added subscribers: grimar, labath.
kwk added a comment.

@grimar @labath  tests did pass locally. Are you fine with this change then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69254



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

[Lldb-commits] [PATCH] D69254: [lldb] drop .symtab removal in minidebuginfo tests

[George Rimar via Phabricator via lldb-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69254



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

[Lldb-commits] [PATCH] D69214: remove multi-argument form of PythonObject::Reset()

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/scripts/Python/python-extensions.swig:683-686
 if (desc_len > 0)
-return lldb_private::PythonString(llvm::StringRef(desc, 
desc_len)).release();
+return PythonString(llvm::StringRef(desc, 
desc_len)).release();
 else
+return PythonString("").release();

You don't have to do this if you don't want, but I couldn't help noticing that 
the "else" branch is completely unnecessary in all of these scriptlets.



Comment at: lldb/scripts/Python/python-typemaps.swig:4
 %typemap(in) char ** {
-  using namespace lldb_private;
+
   /* Check if is a list  */

Could you delete these empty lines too.  You don't have to delete absolutely 
all of them, if you think they're useful for better visual separation, but I 
definitely don't think empty lines at the very beginning of a block are useful.



Comment at: lldb/scripts/Python/python-wrapper.swig:193-194
 {
-using namespace lldb_private;
+
 
 if (python_class_name == NULL || python_class_name[0] == '\0' || 
!session_dictionary_name)

These even have a double empty line now. :)



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:261-264
 void PythonBytes::SetBytes(llvm::ArrayRef bytes) {
   const char *data = reinterpret_cast(bytes.data());
-  PyObject *py_bytes = PyBytes_FromStringAndSize(data, bytes.size());
-  PythonObject::Reset(PyRefType::Owned, py_bytes);
+  *this = Take(PyBytes_FromStringAndSize(data, bytes.size()));
 }

lawrence_danna wrote:
> labath wrote:
> > Do I understand correctly that the idea is to eventually replace these 
> > functions with constructor calls too?
> yea
awesome.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1057
+
+  // global is protected by the GIL
+  static PythonScript read_exception(read_exception_script, "read_exception");

lawrence_danna wrote:
> labath wrote:
> > I just realized I don't actually know what this means. Is it that the 
> > object takes the GIL internally? Or that one must have the GIL taken before 
> > he starts to interact with it? Maybe it'd be better to clarify this in the 
> > class description and just delete these comments.
> Almost everything in PythonDataObjects requires you to call it with the GIL 
> already locked.  The only things that don't are the File methods because 
> they're called by things that don't know anything about python.
> 
> Normally when I need to do something like this I would use dispatch_once, or 
> whatever the LLVM version of that is, so you avoid static initializers, but 
> also initialize the thing in a thread-safe way.
> 
> But in this case we already know that this function can only be called by a 
> thread that has the GIL, so it's safe to just initialize a global variable 
> without further synchronization.
Ok, thanks for explaining. The new comment is much clearer now (though I am not 
sure if it's even needed TBH).



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1068-1070
+Twine message =
+Twine(toCString()) + "\n" +
+"Traceback unavailble, an error occurred while reading it:\n";

This won't work. The reason why the Twine class is so efficient, is that it 
establishes links between various temporary objects. Those temporaries will be 
destroyed when the enclosing full expression is evaluated. This means it's 
never safe to store Twine objects as local variables.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1077
+llvm::handleErrors(backtrace.takeError(), [&](PythonException &E) {
+  backtrace2 = E.ReadBacktraceRecursive(limit - 1);
+});

How likely are we to get another exception while trying to retrieve the 
backtrace? I am wondering if all this complexity is worth it...



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h:26-27
 
+using namespace lldb_private::python;
+
 namespace lldb_private {

No using declarations in headers.



Comment at: 
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp:790-793
+  Expected r = As(factorial(5ll));
+  bool ok = (bool)r;
+  ASSERT_TRUE(ok);
+  EXPECT_EQ(r.get(), 120);

EXPECT_THAT_EXPECTED(As(factorial(5ll)), llvm::HasValue(120))



Comment at: 
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp:842-843
+  auto g = As(globals.GetItem("g"));
+  ASSERT_THAT_EXPECTED(g, llvm::Succeeded());
+  EXPECT_EQ(g.get(), "foobarbaz");
 }

HasValue(foobarbaz)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://revi

[Lldb-commits] [lldb] 9129a28 - [lldb] drop .symtab removal in minidebuginfo tests

[Konrad Kleine via lldb-commits]

Author: Konrad Kleine
Date: 2019-10-21T14:11:21Z
New Revision: 9129a281cd5b8b1fb804be1de396de4a42676570

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

LOG: [lldb] drop .symtab removal in minidebuginfo tests

Summary:
After D69041, we no longer have to manually remove the .symtab section
once yaml2obj was run.

Reviewers: espindola, alexshap

Subscribers: emaste, arichardson, MaskRay, lldb-commits

Tags: #lldb

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

llvm-svn: 375415

Added: 


Modified: 
lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml

Removed: 




diff  --git a/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml 
b/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
index cec34b9c6233..938688cdfe61 100644
--- a/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
+++ b/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
@@ -5,11 +5,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj 2>&1 | FileCheck %s
 
 # CHECK: warning: (x86_64) {{.*}}.obj An error occurred while decompression 
the section .gnu_debugdata: lzma_stream_buffer_decode()=lzma error: 
LZMA_DATA_ERROR

diff  --git a/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml 
b/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
index 230ce8bb1c33..e6ebb0381439 100644
--- a/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
+++ b/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
@@ -2,11 +2,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj | FileCheck %s
 
 # CHECK: [ 0] 1 X Code 0x004005b0 0x000f 0x0012 
multiplyByFour

diff  --git a/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml 
b/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
index a127109e991a..63c82baf07e2 100644
--- a/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
+++ b/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
@@ -5,11 +5,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj 2>&1 | FileCheck %s
 
 # CHECK: warning: (x86_64) {{.*}}.obj No LZMA support found for reading 
.gnu_debugdata section



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

[Lldb-commits] [PATCH] D69254: [lldb] drop .symtab removal in minidebuginfo tests

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9129a281cd5b: [lldb] drop .symtab removal in minidebuginfo 
tests (authored by kwk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69254

Files:
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml


Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
@@ -5,11 +5,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj 2>&1 | FileCheck %s
 
 # CHECK: warning: (x86_64) {{.*}}.obj No LZMA support found for reading 
.gnu_debugdata section
Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
@@ -2,11 +2,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj | FileCheck %s
 
 # CHECK: [ 0] 1 X Code 0x004005b0 0x000f 0x0012 
multiplyByFour
Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
@@ -5,11 +5,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj 2>&1 | FileCheck %s
 
 # CHECK: warning: (x86_64) {{.*}}.obj An error occurred while decompression 
the section .gnu_debugdata: lzma_stream_buffer_decode()=lzma error: 
LZMA_DATA_ERROR


Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
@@ -5,11 +5,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj 2>&1 | FileCheck %s
 
 # CHECK: warning: (x86_64) {{.*}}.obj No LZMA support found for reading .gnu_debugdata section
Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
@@ -2,11 +2,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj | FileCheck %s
 
 # CHECK: [ 0] 1 X Code 0x004005b0 0x000f 0x0012 multiplyByFour
Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
@@ -5,11 +5,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj 2>&1 | FileCheck %s
 
 # CHECK: warning: (x86_64) {{.*}}.obj An error occurred while decompression the section .gnu_debugdata: lzma_stream_buffer_decode()=lzma error: LZMA_DATA_ERROR
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D66795: [Mips] Use appropriate private label prefix based on Mips ABI

[Simon Atanasyan via Phabricator via lldb-commits]

atanasyan added a comment.

In D66795#1706011 , @mbrkusanin wrote:

> @echristo @craig.topper @tstellar @dylanmckay @petecoup
>  If there are no objections then I'll split this into llvm, clang and lldb 
> patches and commit them next week.


Mirko, I think you can commit the patch now because we have not got any 
objections.


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

https://reviews.llvm.org/D66795



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

[Lldb-commits] [PATCH] D66795: [Mips] Use appropriate private label prefix based on Mips ABI

[Eric Christopher via Phabricator via lldb-commits]

echristo added a comment.

I kinda want the option to be encoded in the triple for earlier testing of 
linking issues, but for now this is probably OK.


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

https://reviews.llvm.org/D66795



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

[Lldb-commits] [lldb] 7a79e10 - [lldb] Add test for executing static initializers in expression command

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-10-21T16:07:45Z
New Revision: 7a79e10a82e0d5f84385566493823959dc1697b3

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

LOG: [lldb] Add test for executing static initializers in expression command

llvm-svn: 375422

Added: 

lldb/packages/Python/lldbsuite/test/commands/expression/static-initializers/Makefile

lldb/packages/Python/lldbsuite/test/commands/expression/static-initializers/TestStaticInitializers.py

lldb/packages/Python/lldbsuite/test/commands/expression/static-initializers/main.cpp

Modified: 


Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/expression/static-initializers/Makefile
 
b/lldb/packages/Python/lldbsuite/test/commands/expression/static-initializers/Makefile
new file mode 100644
index ..8b20bcb0
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/expression/static-initializers/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/expression/static-initializers/TestStaticInitializers.py
 
b/lldb/packages/Python/lldbsuite/test/commands/expression/static-initializers/TestStaticInitializers.py
new file mode 100644
index ..e350e6ef930f
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/expression/static-initializers/TestStaticInitializers.py
@@ -0,0 +1,31 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class StaticInitializers(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+""" Test a static initializer. """
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
+# We use counter to observe if the initializer was called.
+self.expect("expr counter", substrs=["(int) $", " = 0"])
+self.expect("expr -p -- struct Foo { Foo() { inc_counter(); } }; Foo 
f;")
+self.expect("expr counter", substrs=["(int) $", " = 1"])
+
+def test_failing_init(self):
+""" Test a static initializer that fails to execute. """
+self.build()
+
+lldbutil.run_to_source_breakpoint(self, '// break here',
+lldb.SBFileSpec("main.cpp", False))
+
+# FIXME: This error message is not even remotely helpful.
+self.expect("expr -p -- struct Foo2 { Foo2() { do_abort(); } }; Foo2 
f;", error=True,
+substrs=["error: couldn't run static initializers: 
couldn't run static initializer:"])

diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/expression/static-initializers/main.cpp
 
b/lldb/packages/Python/lldbsuite/test/commands/expression/static-initializers/main.cpp
new file mode 100644
index ..0bcf1eb3edaf
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/expression/static-initializers/main.cpp
@@ -0,0 +1,11 @@
+#include 
+
+int counter = 0;
+
+void inc_counter() { ++counter; }
+
+void do_abort() { abort(); }
+
+int main() {
+  return 0; // break here
+}



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

[Lldb-commits] [PATCH] D69210: [Disassembler] Simplify MCInst predicates

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D69210#1715679 , @vsk wrote:

> Hm, this patch is bugging me.
>
> It looks a bit like instructions are still decoded multiple times in 
> different ways (e.g. in the `Decode` and 
> `CalculateMnemonicOperandsAndComment` methods, which both modify `m_opcode`). 
> Any ideas on whether/how to consolidate these?


I am all for anything that will improve efficiency. This class has evolved over 
time where we started with just the "CalculateMnemonicOperandsAndComment" and 
then many other features (can branch, etc) were later built into the class. I 
don't believe instructions are kept around for long so they typically serve one 
of two purposes:

- disassembly of instruction stream where only 
CalculateMnemonicOperandsAndComment is needed
- inspection of multiple instructions for stepping looking at can branch and 
other information requests

So I am not sure the decoded multiple times in different ways is really 
important unless we do have a costly client that does both 
CalculateMnemonicOperandsAndComment and inspecting of instruction attributes 
(can branch, etc). Again, these objects are created, used and discarded 
currently AFAIK.




Comment at: lldb/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp:87
+//   - Is not a call
+m_does_branch = eLazyBoolYes;
+m_has_delay_slot = eLazyBoolNo;

Why init this to eLazyBoolYes?


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

https://reviews.llvm.org/D69210



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

[Lldb-commits] [lldb] 5827a82 - Unify timeouts in gdbserver tests and ensure they are larger if ASAN is enabled.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2019-10-21T17:19:42Z
New Revision: 5827a82a5870fcb59a9fb34e6891ca0f009d282a

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

LOG: Unify timeouts in gdbserver tests and ensure they are larger if ASAN is 
enabled.

llvm-svn: 375431

Added: 


Modified: 

lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteProcessInfo.py

lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py

lldb/packages/Python/lldbsuite/test/tools/lldb-server/commandline/TestStubReverseConnect.py
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteProcessInfo.py
 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteProcessInfo.py
index 5a3ae926896a..70cc25520608 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteProcessInfo.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteProcessInfo.py
@@ -53,7 +53,7 @@ def attach_commandline_qProcessInfo_reports_correct_pid(self):
 self.add_process_info_collection_packets()
 
 # Run the stream
-context = self.expect_gdbremote_sequence(timeout_seconds=8)
+context = 
self.expect_gdbremote_sequence(timeout_seconds=self._DEFAULT_TIMEOUT)
 self.assertIsNotNone(context)
 
 # Gather process info response

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
index 1d3a63d27b76..0944ba5d0510 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
@@ -52,7 +52,7 @@ def gather_stop_replies_via_qThreadStopInfo(self, 
thread_count):
 self.assertIsNotNone(context)
 
 # Wait until all threads have started.
-threads = self.wait_for_thread_count(thread_count, timeout_seconds=3)
+threads = self.wait_for_thread_count(thread_count, 
timeout_seconds=self._WAIT_TIMEOUT)
 self.assertIsNotNone(threads)
 
 # On Windows, there could be more threads spawned. For example, 
DebugBreakProcess will

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
index a3abe203a987..1b30718f7481 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestLldbGdbServer.py
@@ -639,7 +639,7 @@ def Hg_switches_to_3_threads(self):
 self.run_process_then_stop(run_seconds=1)
 
 # Wait at most x seconds for 3 threads to be present.
-threads = self.wait_for_thread_count(3, timeout_seconds=5)
+threads = self.wait_for_thread_count(3, 
timeout_seconds=self._WAIT_TIMEOUT)
 self.assertEqual(len(threads), 3)
 
 # verify we can $H to each thead, and $qC matches the thread we set.
@@ -735,7 +735,7 @@ def Hc_then_Csignal_signals_correct_thread(self, 
segfault_signo):
 2: "thread_id"}}],
  True)
 
-context = self.expect_gdbremote_sequence(timeout_seconds=10)
+context = 
self.expect_gdbremote_sequence(timeout_seconds=self._DEFAULT_TIMEOUT)
 self.assertIsNotNone(context)
 signo = context.get("signo")
 self.assertEqual(int(signo, 16), segfault_signo)
@@ -771,7 +771,8 @@ def Hc_then_Csignal_signals_correct_thread(self, 
segfault_signo):
 True)
 
 # Run the sequence.
-context = self.expect_gdbremote_sequence(timeout_seconds=10)
+context = self.expect_gdbremote_sequence(
+timeout_seconds=self._DEFAULT_TIMEOUT)
 self.assertIsNotNone(context)
 
 # Ensure the stop signal is the signal we delivered.
@@ -1485,7 +1486,7 @@ def P_and_p_thread_suffix_work(self):
 self.assertIsNotNone(context)
 
 # Wait for 3 threads to be present.
-threads = self.wait_for_thread_count(3, timeout_seconds=5)
+threads = self.wait_for_thread_count(3, 
timeout_seconds=self._WAIT_TIMEOUT)
 self.assertEqual(len(threads), 3)
 
 expected_reg_values = []

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/commandline/TestStubReverseConnect.py
 
b/lldb/packages/Python/lldbsuite/test/tool

[Lldb-commits] [PATCH] D69273: ValueObject: Fix a crash related to children address type computation

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: jingham, clayborg.
Herald added a subscriber: aprantl.

This patch fixes a crash encountered when debugging optimized code. If some
variable has been completely optimized out, but it's value is nonetheless known,
the compiler can replace it with a DWARF expression computing its value. The
evaluating these expressions results in a eValueTypeHostAddress Value object, as
it's contents are computed into an lldb buffer. However, any value that is
obtained by dereferencing pointers in this object should no longer have the
"host" address type.

Lldb had code to account for this, but it was only present in the
ValueObjectVariable class. This wasn't enough when the object being described
was a struct, as then the object holding the actual pointer was a
ValueObjectChild. This caused lldb to dereference the contained pointer in the
context of the host process and crash.

Though I am not an expert on ValueObjects, it seems to me that this children
address type logic should apply to all types of objects (and indeed, applying
applying the same logic to ValueObjectChild fixes the crash). Therefore, I move
this code to the base class, and arrange it to be run everytime the value is
updated.

The test case is a reduced and simplified version of the original debug info
triggering the crash. Originally we were dealing with a local variable, but as
these require a running process to display, I changed it to use a global one
instead.


https://reviews.llvm.org/D69273

Files:
  include/lldb/Core/ValueObject.h
  source/Core/ValueObject.cpp
  source/Core/ValueObjectVariable.cpp
  test/Shell/SymbolFile/DWARF/DW_OP_piece-struct.s

Index: test/Shell/SymbolFile/DWARF/DW_OP_piece-struct.s
===
--- /dev/null
+++ test/Shell/SymbolFile/DWARF/DW_OP_piece-struct.s
@@ -0,0 +1,113 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s
+# RUN: %lldb %t -o "target variable reset" -b | FileCheck %s
+
+# CHECK: (lldb) target variable reset
+# CHECK: (auto_reset) reset = {
+# CHECK:   ptr = 0xdeadbeefbaadf00d
+# CHECK:   prev = false
+# CHECK: }
+
+.section.debug_abbrev,"",@progbits
+.byte   1   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   1   # DW_CHILDREN_yes
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   2   # Abbreviation Code
+.byte   52  # DW_TAG_variable
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   2   # DW_AT_location
+.byte   24  # DW_FORM_exprloc
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   3   # Abbreviation Code
+.byte   36  # DW_TAG_base_type
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   62  # DW_AT_encoding
+.byte   11  # DW_FORM_data1
+.byte   11  # DW_AT_byte_size
+.byte   11  # DW_FORM_data1
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   4   # Abbreviation Code
+.byte   19  # DW_TAG_structure_type
+.byte   1   # DW_CHILDREN_yes
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   11  # DW_AT_byte_size
+.byte   11  # DW_FORM_data1
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   5   # Abbreviation Code
+.byte   13  # DW_TAG_member
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   56  # DW_AT_data_member_location
+.byte   11  # DW_FORM_data1
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   6   # Abbreviation Code
+.byte   15  # DW_TAG_point

[Lldb-commits] [PATCH] D69273: ValueObject: Fix a crash related to children address type computation

[Greg Clayton via Phabricator via lldb-commits]

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

Looks good!


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

https://reviews.llvm.org/D69273



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

[Lldb-commits] [PATCH] D69210: [Disassembler] Simplify MCInst predicates

[Vedant Kumar via Phabricator via lldb-commits]

vsk marked an inline comment as done.
vsk added a comment.

In D69210#1716861 , @clayborg wrote:

> In D69210#1715679 , @vsk wrote:
>
> > Hm, this patch is bugging me.
> >
> > It looks a bit like instructions are still decoded multiple times in 
> > different ways (e.g. in the `Decode` and 
> > `CalculateMnemonicOperandsAndComment` methods, which both modify 
> > `m_opcode`). Any ideas on whether/how to consolidate these?
>
>
> I am all for anything that will improve efficiency. This class has evolved 
> over time where we started with just the 
> "CalculateMnemonicOperandsAndComment" and then many other features (can 
> branch, etc) were later built into the class. I don't believe instructions 
> are kept around for long so they typically serve one of two purposes:
>
> - disassembly of instruction stream where only 
> CalculateMnemonicOperandsAndComment is needed
> - inspection of multiple instructions for stepping looking at can branch and 
> other information requests
>
>   So I am not sure the decoded multiple times in different ways is really 
> important unless we do have a costly client that does both 
> CalculateMnemonicOperandsAndComment and inspecting of instruction attributes 
> (can branch, etc). Again, these objects are created, used and discarded 
> currently AFAIK.


Thanks for your comment Greg. Let me try and restate the issue I see as my 
concern isn't about performance.

It looks like `Decode` and `CalculateMnemonicOperandsAndComment` mutate 
`m_opcode` in different ways. Separately, the predicates read `m_opcode`. So 
I'm not sure whether/in-which-order the mutating methods need to be run before 
the predicates can safely be called. I'd like to consolidate all the code that 
mutates `m_opcode` in one place, to make the predicates always safe to call. 
Does that seem reasonable? Or am I overthinking something?




Comment at: lldb/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp:87
+//   - Is not a call
+m_does_branch = eLazyBoolYes;
+m_has_delay_slot = eLazyBoolNo;

clayborg wrote:
> Why init this to eLazyBoolYes?
I believe this preserves the existing behavior of the class. InstructionLLVMC 
conservatively says that instructions can branch, in the absence of information.


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

https://reviews.llvm.org/D69210



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

[Lldb-commits] [PATCH] D69210: [Disassembler] Simplify MCInst predicates

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D69210#1717042 , @vsk wrote:

> In D69210#1716861 , @clayborg wrote:
>
> > In D69210#1715679 , @vsk wrote:
> >
> > > Hm, this patch is bugging me.
> > >
> > > It looks a bit like instructions are still decoded multiple times in 
> > > different ways (e.g. in the `Decode` and 
> > > `CalculateMnemonicOperandsAndComment` methods, which both modify 
> > > `m_opcode`). Any ideas on whether/how to consolidate these?
> >
> >
> > I am all for anything that will improve efficiency. This class has evolved 
> > over time where we started with just the 
> > "CalculateMnemonicOperandsAndComment" and then many other features (can 
> > branch, etc) were later built into the class. I don't believe instructions 
> > are kept around for long so they typically serve one of two purposes:
> >
> > - disassembly of instruction stream where only 
> > CalculateMnemonicOperandsAndComment is needed
> > - inspection of multiple instructions for stepping looking at can branch 
> > and other information requests
> >
> >   So I am not sure the decoded multiple times in different ways is really 
> > important unless we do have a costly client that does both 
> > CalculateMnemonicOperandsAndComment and inspecting of instruction 
> > attributes (can branch, etc). Again, these objects are created, used and 
> > discarded currently AFAIK.
>
>
> Thanks for your comment Greg. Let me try and restate the issue I see as my 
> concern isn't about performance.
>
> It looks like `Decode` and `CalculateMnemonicOperandsAndComment` mutate 
> `m_opcode` in different ways. Separately, the predicates read `m_opcode`. So 
> I'm not sure whether/in-which-order the mutating methods need to be run 
> before the predicates can safely be called. I'd like to consolidate all the 
> code that mutates `m_opcode` in one place, to make the predicates always safe 
> to call. Does that seem reasonable? Or am I overthinking something?


It seems that CalculateMnemonicOperandsAndComment only mutates m_opcode when 
the instruction size returned by:

  size_t inst_size = mc_disasm_ptr->GetMCInst(opcode_data, opcode_data_len, pc, 
inst);

is zero. It also is unclear to me that the mutating calls in 
CalculateMnemonicOperandsAndComment really do anything? They decode a value 
from the data, then then put them back into m_opcode? Also, if "inst_size" is 
zero in decode:

  const size_t inst_size =
  mc_disasm_ptr->GetMCInst(opcode_data, opcode_data_len, pc, inst);
  if (inst_size == 0)
m_opcode.Clear();
  else {
m_opcode.SetOpcodeBytes(opcode_data, inst_size);
m_is_valid = true;
  }

Then m_opcode.Clear() is called and m_opcode won't contain anything, so I am 
guess only architectures with fixed opcode sizes will be able to show ".long" 
or any of that kind of stuff? And only those will trigger mutating the opcode 
value in CalculateMnemonicOperandsAndComment?


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

https://reviews.llvm.org/D69210



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

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

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

https://reviews.llvm.org/D68961



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

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D68961#1714241 , @labath wrote:

> In D68961#1713566 , @shafik wrote:
>
> > We also have the lldb-cmake-matrix 
> >  bot which 
> > runs the test suite using older versions of clang which is there for 
> > exactly this purpose to catch regressions due to features not supported by 
> > older compiler versions. Which would catch any regressions here.
>
>
> I'm afraid I have to disagree with that. Like I said the last time  > when this topic came up, I don't think 
> that this "matrix" testing is the way to test dwarf features. The "matrix" 
> bot is very useful to catch regressions in parsing old debug info (because 
> there will always be some quirk that you forget to think of), but I don't 
> think there should be any piece of code that is _only_ tested this way, 
> particularly when it is very easy to do so (like it is here).
>
> As you're probably aware, there's an "end-to-end testing" discussion going on 
> on the mailing lists right now. One of the main questions being debated is 
> whether it is ok to have a test which checks the assembly generated from some 
> c(++) source code. And there's a lot of pushback saying that this is testing 
> too much. However, even such a test is peanuts compared to 
> clang-ast-from-dwarf-unamed-and-anon-structs.cpp. Even though it's one of the 
> simplest lldb test we have, it runs the compiler front end, back end *and* 
> assembler, and then reads the output hoping that the pipeline has generated 
> the thing it wants to test. (And btw, it will fail on windows.)  It doesn't 
> even check that the compiler has generated the thing it wants to check, so if 
> the compiler output later changes to not include DW_AT_export_symbols, it 
> will start to test something completely different, leaving the code added 
> here uncovered, unless someone remembers to add a new row to the matrix bot.
>
> Not that this is very likely to happen in this particular case, but these 
> kinds of changes happen all the time. Just last week, llvm started generating 
> a very different style of location lists. Both lldb  > and dsymutil  > had to be fixed to handle that. Both 
> patches had tests, which were not based on running clang to generate the 
> debug info (the dsymutil test had a checked in binary, which I am not a fan 
> of, but that's a different story). Even so, it's very likely that this has 
> regressed our coverage of the previous forms of location lists because we had 
> no tests (except some basic ones I've added two or three months ago) which 
> tested this code directly. And we only know about this because the initial 
> implementation was wrong/incomplete, so we noticed the breakage.
>
> So, overall, I still think we should have an even more targeted test here. 
> Maybe it's not fair to ask you to add this kind of a test for the "old" style 
> structs because that is past and not really "on you". However, I think we 
> should be trying to add these kinds of tests for new features whenever is 
> possible (another thing -- I imagine debugging failures from the matrix bot 
> can be hard, though I've fortunately had not had to do that yet). I've been 
> trying to do that for the past year, and while it takes a while to get used 
> to, after some time I've gotten fairly efficient at generating/reducing "raw" 
> dwarf. Besides being able to generate "stable" tests, this also enables one 
> to generate tests for "incorrect" dwarf, which is one of the things which is 
> impossible with the "clang -g" approach.


When the I added the feature to the front end tests were added to verify that 
`DW_AT_export_symbols` is being generated for anonymous structs in D66605 
 and D7  
so if this regresses in the front-end we will catch it vis these tests. So as 
far I can tell we have tests at every point it can regress.


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

https://reviews.llvm.org/D68961



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

[Lldb-commits] [PATCH] D69214: remove multi-argument form of PythonObject::Reset()

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added inline comments.



Comment at: lldb/scripts/Python/python-extensions.swig:683-686
 if (desc_len > 0)
-return lldb_private::PythonString(llvm::StringRef(desc, 
desc_len)).release();
+return PythonString(llvm::StringRef(desc, 
desc_len)).release();
 else
+return PythonString("").release();

labath wrote:
> You don't have to do this if you don't want, but I couldn't help noticing 
> that the "else" branch is completely unnecessary in all of these scriptlets.
good point.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1077
+llvm::handleErrors(backtrace.takeError(), [&](PythonException &E) {
+  backtrace2 = E.ReadBacktraceRecursive(limit - 1);
+});

labath wrote:
> How likely are we to get another exception while trying to retrieve the 
> backtrace? I am wondering if all this complexity is worth it...
It's actually not that unusual that someone writes a `__str__` method for their 
exception which is broken and also throws an exception.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1077
+llvm::handleErrors(backtrace.takeError(), [&](PythonException &E) {
+  backtrace2 = E.ReadBacktraceRecursive(limit - 1);
+});

lawrence_danna wrote:
> labath wrote:
> > How likely are we to get another exception while trying to retrieve the 
> > backtrace? I am wondering if all this complexity is worth it...
> It's actually not that unusual that someone writes a `__str__` method for 
> their exception which is broken and also throws an exception.
actually, it looks like `traceback.print_exception` already takes care of that 
situation, so you're right it's not very useful.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h:26-27
 
+using namespace lldb_private::python;
+
 namespace lldb_private {

labath wrote:
> No using declarations in headers.
even an "Impl" header?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69214



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

[Lldb-commits] [PATCH] D69214: remove multi-argument form of PythonObject::Reset()

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 225942.
lawrence_danna marked 14 inline comments as done.
lawrence_danna added a comment.

fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69214

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/scripts/Python/python-extensions.swig
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/Python/python-wrapper.swig
  lldb/scripts/lldb.swig
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonExceptionStateTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonExceptionStateTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonExceptionStateTests.cpp
+++ /dev/null
@@ -1,164 +0,0 @@
-//===-- PythonExceptionStateTest.cpp --*- C++
-//-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "gtest/gtest.h"
-
-#include "Plugins/ScriptInterpreter/Python/PythonDataObjects.h"
-#include "Plugins/ScriptInterpreter/Python/PythonExceptionState.h"
-#include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h"
-#include "Plugins/ScriptInterpreter/Python/lldb-python.h"
-
-#include "PythonTestSuite.h"
-
-using namespace lldb_private;
-
-class PythonExceptionStateTest : public PythonTestSuite {
-public:
-protected:
-  void RaiseException() {
-PyErr_SetString(PyExc_RuntimeError, "PythonExceptionStateTest test error");
-  }
-};
-
-TEST_F(PythonExceptionStateTest, TestExceptionStateChecking) {
-  PyErr_Clear();
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  RaiseException();
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestAcquisitionSemantics) {
-  PyErr_Clear();
-  PythonExceptionState no_error(false);
-  EXPECT_FALSE(no_error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-  RaiseException();
-  PythonExceptionState error(false);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  error.Discard();
-
-  PyErr_Clear();
-  RaiseException();
-  error.Acquire(false);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestDiscardSemantics) {
-  PyErr_Clear();
-
-  // Test that discarding an exception does not restore the exception
-  // state even when auto-restore==true is set
-  RaiseException();
-  PythonExceptionState error(true);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  error.Discard();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-}
-
-TEST_F(PythonExceptionStateTest, TestResetSemantics) {
-  PyErr_Clear();
-
-  // Resetting when auto-restore is true should restore.
-  RaiseException();
-  PythonExceptionState error(true);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  error.Reset();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-
-  // Resetting when auto-restore is false should discard.
-  RaiseException();
-  PythonExceptionState error2(false);
-  EXPECT_TRUE(error2.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  error2.Reset();
-  EXPECT_FALSE(error2.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestManualRestoreSemantics) {
-  PyErr_Clear();
-  RaiseException();
-  PythonExceptionState error(false);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  error.Restore();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestAutoRestoreSemantics) {
-  PyErr_Clear();
-  // Test that using the auto-restore flag correctly restores the exception
-  // state on destru

[Lldb-commits] [PATCH] D68856: convert SBDebugger::***FileHandle() wrappers to native files.

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

I'm afraid this causes a few test failures on NetBSD, e.g.: 
http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22255/steps/run%20unit%20tests/logs/FAIL%3A%20lldb-api%3A%3ATestDefaultConstructorForAPIObjects.py

Could you help me debugging it? I haven't been working on the Python API that 
deep, so I'd use any help I can get. For a start, a way to make that error 
message point to a specific operation would be helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68856



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

[Lldb-commits] [PATCH] D69100: COFF: Create a separate "section" for the file header

[Martin Storsjö via Phabricator via lldb-commits]

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

I've happened to poke around a bit more in this piece of code now recently, and 
it does indeed seem reasonable.


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

https://reviews.llvm.org/D69100



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

[Lldb-commits] [PATCH] D68856: convert SBDebugger::***FileHandle() wrappers to native files.

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

In D68856#1717207 , @mgorny wrote:

> I'm afraid this causes a few test failures on NetBSD, e.g.: 
> http://lab.llvm.org:8011/builders/netbsd-amd64/builds/22255/steps/run%20unit%20tests/logs/FAIL%3A%20lldb-api%3A%3ATestDefaultConstructorForAPIObjects.py
>
> Could you help me debugging it? I haven't been working on the Python API that 
> deep, so I'd use any help I can get. For a start, a way to make that error 
> message point to a specific operation would be helpful.


sure, um do you have a netbsd machine handy?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68856



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

[Lldb-commits] [PATCH] D68968: [android/process info] Introduce display_name

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 225947.
wallace retitled this revision from "[android/process info] Introduce bundle 
id" to "[android/process info] Introduce display_name".
wallace edited the summary of this revision.
wallace added a comment.
Herald added a subscriber: kristof.beyls.

now using display_name instead of bundle_id. I've also changed the diff title 
and description


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68968

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/ProcessInfo.h
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
  lldb/source/Host/linux/Host.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Utility/ProcessInfo.cpp

Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -38,11 +38,18 @@
   m_pid = LLDB_INVALID_PROCESS_ID;
 }
 
+// On systems like android, there are cases in which a process name can
+// correspond to a bundle id (e.g. com.test.app), which is not an executable
+// path.
 const char *ProcessInfo::GetName() const {
+  if (!m_display_name.empty())
+return m_display_name.c_str();
   return m_executable.GetFilename().GetCString();
 }
 
 llvm::StringRef ProcessInfo::GetNameAsStringRef() const {
+  if (!m_display_name.empty())
+return m_display_name;
   return m_executable.GetFilename().GetStringRef();
 }
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -1187,14 +1187,16 @@
   response.PutCString("name:");
   response.PutStringAsRawHex8(proc_info.GetExecutableFile().GetCString());
 
-  response.PutChar(';');
-  response.PutCString("args:");
+  response.PutCString(";args:");
   response.PutStringAsRawHex8(proc_info.GetArg0());
   for (auto &arg : proc_info.GetArguments()) {
 response.PutChar('-');
 response.PutStringAsRawHex8(arg.ref());
   }
 
+  response.PutCString(";display_name:");
+  response.PutStringAsRawHex8(proc_info.GetDisplayName());
+
   response.PutChar(';');
   const ArchSpec &proc_arch = proc_info.GetArchitecture();
   if (proc_arch.IsValid()) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1948,6 +1948,11 @@
 process_info.GetArguments().AppendArgument(arg);
   is_arg0 = false;
 }
+  } else if (name.equals("display_name")) {
+StringExtractor extractor(value);
+std::string display_name;
+extractor.GetHexByteString(display_name);
+process_info.SetDisplayName(display_name);
   } else if (name.equals("cputype")) {
 value.getAsInteger(0, cpu);
   } else if (name.equals("cpusubtype")) {
Index: lldb/source/Host/linux/Host.cpp
===
--- lldb/source/Host/linux/Host.cpp
+++ lldb/source/Host/linux/Host.cpp
@@ -202,6 +202,44 @@
   }
 }
 
+static bool GetProcessNameFromStat(::pid_t pid, std::string& name) {
+  auto BufferOrError = getProcFile(pid, "stat");
+  if (!BufferOrError)
+return false;
+
+  std::unique_ptr Stat = std::move(*BufferOrError);
+  llvm::StringRef Rest = Stat->getBuffer();
+  if (Rest.empty())
+return false;
+
+  size_t from = Rest.find_first_of('(');
+  size_t to = Rest.find_first_of(')');
+  if (from == std::string::npos || to == std::string::npos)
+return false;
+  from++;
+  name = Rest.substr(from, to - from);
+  return true;
+}
+
+static void SetProcessDisplayName(::pid_t pid, ProcessInstanceInfo& process_info) {
+  if (!process_info.GetNameAsStringRef().empty())
+return;
+  if (!process_info.GetArg0().empty()) {
+  // When /proc//exe is not readable, then we use arg0 as display name.
+  // On Android devices this might contain the apk package name.
+  process_info.SetDisplayName(process_info.GetArg0());
+  return;
+  }
+  // If /proc//cmdline is not readable, then it might correspond to a kernel thread
+  // or system process. We fallback to reading from /proc/stat for a display name.
+  std::string name;
+  if (GetProcessNameFromStat(pid, name)) {
+// We follow the guideline of surrounding the name with brackets, as explained here
+// https://unix.stackexchange.com/questions/22121/what-do-the-brackets-around-processes-m

[Lldb-commits] [PATCH] D69214: remove multi-argument form of PythonObject::Reset()

[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/scripts/Python/python-wrapper.swig:226
 {
-using namespace lldb_private;
 
 if (python_class_name == NULL || python_class_name[0] == '\0' || 
!session_dictionary_name)

There are empty lines from this point on in this file.  Before they made sense 
because they were separating the using directive from the actual code, but now 
they've just become useless empty lines at the start of a block. Please delete 
them.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h:26-27
 
+using namespace lldb_private::python;
+
 namespace lldb_private {

lawrence_danna wrote:
> labath wrote:
> > No using declarations in headers.
> even an "Impl" header?
Nope. The "impl" here comes from the class name, but this is still a header 
like any other. Theoretically you could also put everything declared in this 
header into the python namespace, but otoh we have to stop somewhere, and this 
patch has gotten quite big already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69214



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

[Lldb-commits] [PATCH] D69100: COFF: Create a separate "section" for the file header

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth accepted this revision.
amccarth added a comment.

I'm OK with this.  I'm just wondering whether it would be a good idea to make 
it clear that these header sections are "not considered to be a section in the 
strictest sense."  Is the distinction going to be important to future code 
readers?  Do we already have "sections" that aren't truly "sections"?

Perhaps just a comment where the header sections are created?


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

https://reviews.llvm.org/D69100



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

[Lldb-commits] [PATCH] D68856: convert SBDebugger::***FileHandle() wrappers to native files.

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D68856#1717247 , @lawrence_danna 
wrote:

> sure, um do you have a netbsd machine handy?


If you mean one I could you access to, then I'm afraid not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68856



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

[Lldb-commits] [lldb] ed870cc - Found more timeouts to unify.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2019-10-21T20:50:45Z
New Revision: ed870cce676ec873d5d0c9e084744ffba0eb67fc

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

LOG: Found more timeouts to unify.

llvm-svn: 375454

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteKill.py

lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py

lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteKill.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteKill.py
index 54d72907a0e5..ece0cd16393a 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteKill.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteKill.py
@@ -32,7 +32,7 @@ def attach_commandline_kill_after_initial_stop(self):
 
 # Wait a moment for completed and now-detached inferior process to
 # clear.
-time.sleep(1)
+time.sleep(self._WAIT_TIMEOUT)
 
 if not lldb.remote_platform:
 # Process should be dead now. Reap results.

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
index ed60244e88f1..4effc154d2d7 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
@@ -43,7 +43,7 @@ def gather_stop_reply_fields(self, post_startup_log_lines, 
thread_count,
 hw_info = self.parse_hw_info(context)
 
 # Give threads time to start up, then break.
-time.sleep(1)
+time.sleep(self._WAIT_TIMEOUT)
 self.reset_test_sequence()
 self.test_sequence.add_log_lines(
 [
@@ -61,7 +61,8 @@ def gather_stop_reply_fields(self, post_startup_log_lines, 
thread_count,
 self.assertIsNotNone(context)
 
 # Wait until all threads have started.
-threads = self.wait_for_thread_count(thread_count, timeout_seconds=3)
+threads = self.wait_for_thread_count(thread_count,
+ 
timeout_seconds=self._WAIT_TIMEOUT)
 self.assertIsNotNone(threads)
 self.assertEqual(len(threads), thread_count)
 

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
index 0944ba5d0510..ffd56e3852bc 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
@@ -34,7 +34,7 @@ def gather_stop_replies_via_qThreadStopInfo(self, 
thread_count):
 self.assertIsNotNone(context)
 
 # Give threads time to start up, then break.
-time.sleep(1)
+time.sleep(self._WAIT_TIMEOUT)
 self.reset_test_sequence()
 self.test_sequence.add_log_lines(
 [
@@ -52,7 +52,8 @@ def gather_stop_replies_via_qThreadStopInfo(self, 
thread_count):
 self.assertIsNotNone(context)
 
 # Wait until all threads have started.
-threads = self.wait_for_thread_count(thread_count, 
timeout_seconds=self._WAIT_TIMEOUT)
+threads = self.wait_for_thread_count(thread_count,
+ 
timeout_seconds=self._WAIT_TIMEOUT)
 self.assertIsNotNone(threads)
 
 # On Windows, there could be more threads spawned. For example, 
DebugBreakProcess will



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

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68961#1717197 , @shafik wrote:

> When the I added the feature to the front end tests were added to verify that 
> `DW_AT_export_symbols` is being generated for anonymous structs in D66605 
>  and D7 
>  so if this regresses in the front-end we 
> will catch it vis these tests. So as far I can tell we have tests at every 
> point it can regress.


But that's a test for clang. It will make sure it clang does stop emitting this 
attribute accidentally, but it will not help you if the removal is a conscious 
decision. At that point, the clang test will be deleted/modified too, but I 
doubt anyone will think of lldb and the fact that some part of lldb codebase 
becomes untested.

In fact, I think these patches illustrate very well the point I'm trying to 
make. D7  does not check that the 
attribute ends up in the debug info. It only adds a test to ensure that clang 
emits some llvm IR. It could test the actual dwarf, but it doesn't, because 
llvm has a strong preference for single-component unit tests. lldb's test suite 
is an exception in the llvm world in that nearly every lldb test is an 
end-to-end test.


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

https://reviews.llvm.org/D68961



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

[Lldb-commits] [PATCH] D69100: COFF: Create a separate "section" for the file header

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D69100#1717251 , @amccarth wrote:

> I'm OK with this.  I'm just wondering whether it would be a good idea to make 
> it clear that these header sections are "not considered to be a section in 
> the strictest sense."  Is the distinction going to be important to future 
> code readers?  Do we already have "sections" that aren't truly "sections"?
>
> Perhaps just a comment where the header sections are created?


Both ELF and MachO have the concept of a "segment", which is the thing which 
describes how objects are loaded into memory. These are also represented as 
"sections" in lldb (and real sections are their "subsections"). However, this 
is the only case where we're inventing a "section" out of thin air, but that 
seems reasonable as neither elf nor mach-o have a concept of a piece of file 
which is loaded into memory, but is not a part of any section/segment.

So yeah, sure, adding a comment sounds like a good idea.


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

https://reviews.llvm.org/D69100



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

[Lldb-commits] [PATCH] D68856: convert SBDebugger::***FileHandle() wrappers to native files.

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

In D68856#1717257 , @mgorny wrote:

> In D68856#1717247 , @lawrence_danna 
> wrote:
>
> > sure, um do you have a netbsd machine handy?
>
>
> If you mean one I could you access to, then I'm afraid not.


ok, i've got a VM, how do I install git on this thing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68856



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

[Lldb-commits] [lldb] 3434472 - XFAIL TestLocalVariables.py on Windows

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2019-10-21T22:16:28Z
New Revision: 3434472ed74141848634b5eb3cd625d651e22562

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

LOG: XFAIL TestLocalVariables.py on Windows

This test has been failing for a while on the Windows bot.

https://bugs.llvm.org/show_bug.cgi?id=43752

llvm-svn: 375459

Added: 


Modified: 

lldb/packages/Python/lldbsuite/test/lang/c/local_variables/TestLocalVariables.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/lang/c/local_variables/TestLocalVariables.py
 
b/lldb/packages/Python/lldbsuite/test/lang/c/local_variables/TestLocalVariables.py
index 7071b675f3bc..b1805f4a8b40 100644
--- 
a/lldb/packages/Python/lldbsuite/test/lang/c/local_variables/TestLocalVariables.py
+++ 
b/lldb/packages/Python/lldbsuite/test/lang/c/local_variables/TestLocalVariables.py
@@ -25,6 +25,7 @@ def setUp(self):
 self.line = line_number(
 self.source, '// Set break point at this line.')
 
+@skipIfWindows
 def test_c_local_variables(self):
 """Test local variable value."""
 self.build()



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

[Lldb-commits] [PATCH] D68856: convert SBDebugger::***FileHandle() wrappers to native files.

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

I spoke too soon, I don't have a *usable* netbsd VM.

@mgorny, Do you have a machine you can debug on yourself?  Do those errors 
happen if you run the same test on an interactive terminal?   It could be the 
stdin stream is not set to a valid file descriptor for the CI run.Can you 
step through the SWIG wrappers and tell me where it errors out?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68856



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

[Lldb-commits] [lldb] 667c2eb - Factor out common test functionality into a helper class. (NFC)

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2019-10-21T22:46:53Z
New Revision: 667c2eb08b92907b260aee64a25250f610fcdc30

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

LOG: Factor out common test functionality into a helper class. (NFC)

llvm-svn: 375464

Added: 


Modified: 
lldb/unittests/Expression/DWARFExpressionTest.cpp

Removed: 




diff  --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp 
b/lldb/unittests/Expression/DWARFExpressionTest.cpp
index 291aa4d8ba54..f726d7766841 100644
--- a/lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -22,6 +22,23 @@
 
 using namespace lldb_private;
 
+static llvm::Expected Evaluate(llvm::ArrayRef expr,
+   lldb::ModuleSP module_sp = {},
+   DWARFUnit *unit = nullptr) {
+  DataExtractor extractor(expr.data(), expr.size(), lldb::eByteOrderLittle,
+  /*addr_size*/ 4);
+  Value result;
+  Status status;
+  if (!DWARFExpression::Evaluate(
+  /*exe_ctx*/ nullptr, /*reg_ctx*/ nullptr, module_sp, extractor, unit,
+  lldb::eRegisterKindLLDB,
+  /*initial_value_ptr*/ nullptr,
+  /*object_address_ptr*/ nullptr, result, &status))
+return status.ToError();
+
+  return result.GetScalar();
+}
+
 /// A mock module holding an object file parsed from YAML.
 class YAMLModule : public lldb_private::Module {
 public:
@@ -99,22 +116,51 @@ class YAMLObjectFile : public lldb_private::ObjectFile {
   /// \}
 };
 
-static llvm::Expected Evaluate(llvm::ArrayRef expr,
-   lldb::ModuleSP module_sp = {},
-   DWARFUnit *unit = nullptr) {
-  DataExtractor extractor(expr.data(), expr.size(), lldb::eByteOrderLittle,
-  /*addr_size*/ 4);
-  Value result;
-  Status status;
-  if (!DWARFExpression::Evaluate(
-  /*exe_ctx*/ nullptr, /*reg_ctx*/ nullptr, module_sp, extractor, unit,
-  lldb::eRegisterKindLLDB,
-  /*initial_value_ptr*/ nullptr,
-  /*object_address_ptr*/ nullptr, result, &status))
-return status.ToError();
+/// Helper class that can construct a module from YAML and evaluate
+/// DWARF expressions on it.
+class YAMLModuleTester {
+  llvm::StringMap> m_sections_map;
+  lldb::ModuleSP m_module_sp;
+  lldb::ObjectFileSP m_objfile_sp;
+  DWARFUnitSP m_dwarf_unit;
+  std::unique_ptr m_symfile_dwarf;
 
-  return result.GetScalar();
-}
+public:
+  /// Parse the debug info sections from the YAML description.
+  YAMLModuleTester(llvm::StringRef yaml_data, llvm::StringRef triple) {
+FileSystem::Initialize();
+
+auto sections_map = llvm::DWARFYAML::EmitDebugSections(yaml_data, true);
+if (!sections_map)
+  return;
+m_sections_map = std::move(*sections_map);
+ArchSpec arch(triple);
+m_module_sp = std::make_shared(arch);
+m_objfile_sp = std::make_shared(m_module_sp, 
m_sections_map);
+static_cast(m_module_sp.get())->SetObjectFile(m_objfile_sp);
+
+lldb::user_id_t uid = 0;
+llvm::StringRef raw_debug_info = m_sections_map["debug_info"]->getBuffer();
+lldb_private::DataExtractor debug_info(
+raw_debug_info.data(), raw_debug_info.size(),
+m_objfile_sp->GetByteOrder(), m_objfile_sp->GetAddressByteSize());
+lldb::offset_t offset_ptr = 0;
+m_symfile_dwarf = std::make_unique(m_objfile_sp, nullptr);
+llvm::Expected dwarf_unit = DWARFUnit::extract(
+*m_symfile_dwarf, uid,
+*static_cast(&debug_info),
+DIERef::DebugInfo, &offset_ptr);
+if (dwarf_unit)
+  m_dwarf_unit = dwarf_unit.get();
+  }
+  ~YAMLModuleTester() { FileSystem::Terminate(); }
+  DWARFUnitSP GetDwarfUnit() { return m_dwarf_unit; }
+
+  // Evaluate a raw DWARF expression.
+  llvm::Expected Eval(llvm::ArrayRef expr) {
+return ::Evaluate(expr, m_module_sp, m_dwarf_unit.get());
+  }
+};
 
 /// Unfortunately Scalar's operator==() is really picky.
 static Scalar GetScalar(unsigned bits, uint64_t value, bool sign) {
@@ -226,78 +272,49 @@ TEST(DWARFExpression, DW_OP_convert) {
   uint8_t offs_uchar = 0x0017;
   uint8_t offs_schar = 0x001a;
 
-  //
-  // Setup. Parse the debug info sections from the YAML description.
-  //
-  auto sections_map = llvm::DWARFYAML::EmitDebugSections(yamldata, true);
-  ASSERT_TRUE((bool)sections_map);
-  ArchSpec arch("i386-unknown-linux");
-  FileSystem::Initialize();
-  auto module_sp = std::make_shared(arch);
-  lldb::ObjectFileSP objfile_sp =
-  std::make_shared(module_sp, *sections_map);
-  module_sp->SetObjectFile(objfile_sp);
-  SymbolFileDWARF symfile_dwarf(objfile_sp, nullptr);
-
-  lldb::user_id_t uid = 0;
-  llvm::StringRef raw_debug_info = (*sections_map)[

[Lldb-commits] [PATCH] D69230: RFC: specialized Optional for T that can represent its own invalid state

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: dblaikie.
labath added a subscriber: dblaikie.
labath added a comment.

Lets loop in @dblaikie for start, and see how it goes from there. You can also 
check who modified/reviewed changes to this file lately..

I am a very big opponent of IsValid() methods, and so I am very much in favour 
of this idea. However, I expect it to be moderately controversial (which is 
part of the reason why I haven't tried proposing anything like that yet).

The thing I would like to see here is to have this behaviour be configurable 
via a traits argument of the Optional class, similarly to how DenseMap allows 
the type to specify default traits by specialising DenseMapInfo (I recommend 
taking a look at that for inspiration/consistency), but then a specific user 
can also override that and use a custom traits class. That would be 
particularly useful for builtin types -- one cannot pick any uint32_t value as 
an "invalid value" in general, but for a particular application, there usually 
is one free value. This would allow us to replace all the `pid == 
LLDB_INVALID_PROCESS_ID` checks in lldb with this new optional class.

Another thing to consider is that llvm::Optional tries to follow the interface 
of the (upcoming) std::optional, which does not have this configurability. This 
would put it in the same category as DenseMap (DenseOptional :P ?), which 
matches the interface (mostly) but has our own specific optimizations.

All that said, I don't think this needs to block your work on the python 
classes in lldb. IIUC, all/most of them are just temporaries anyway, and so 
their size does not matter that much (and can be often optimized away).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69230



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

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

[Vedant Kumar via Phabricator via lldb-commits]

vsk marked an inline comment as done.
vsk added a comment.

In D69148#1715532 , @labath wrote:

> In D69148#1714785 , @vsk wrote:
>
> > The death tests are flaky. I've noticed two issues:
> >
> > 1. When run under lit, the DisableExitOnSIGPIPE doesn't actually exit when 
> > it receives SIGPIPE. Dtrace suggests that the unit test process inherits an 
> > "ignore" sigaction from its parent.
> > 2. When run in parallel, exiting causes the unit test to run atexit 
> > destructors for other unit tests lumped into the SupportTests binary. One 
> > of these is a condition_variable destructor and it hangs. Also, gtest 
> > warns: `Death tests use fork(), which is unsafe particularly in a threaded 
> > context. For this test, Google Test detected 21 threads.`
> >
> >   So I plan to give up on testing "EnableExitOnSIGPIPE" and will write a 
> > non-death test to get some coverage.
>
>
> gtest gives you a (somewhat rudimentary) mechanism of ensuring death tests 
> run in a single threaded fashion. It consists of tacking "DeathTest" at the 
> end of your test case name (like `TEST(SignalDeathTest, Whatever)`). These 
> kinds of tests are run first, hopefully before the tests which spin up 
> various other threads.


A problem I'm encountering with this is that the static initializer from 
Signals.inc don't appear to be re-run between death tests. This makes the death 
tests pretty brittle, imo, as swapping the order of the tests can break them. 
Do you think the extra coverage is worth it?

> As for the parent "ignore" action (it's more likely to be a signal mask, 
> actually), this can be reset with an appropriate sigprocmask(2) call.

Thanks, that's a great catch.




Comment at: lldb/tools/driver/Driver.cpp:850
 #if !defined(_MSC_VER)
   signal(SIGPIPE, SIG_IGN);
   signal(SIGWINCH, sigwinch_handler);

labath wrote:
> I don't think this line does anything anymore.
I don't think that's true. LLVM still does `registerHandler(SIGPIPE, 
SignalKind::IsKill)` in `RegisterHandlers`. At least, removing the 
`signal(SIGPIPE, SIG_IGN)` calls causes the unit test to "fail" (i.e. the test 
RUNs but doesn't reach OK).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69148



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

[Lldb-commits] [PATCH] D69230: RFC: specialized Optional for T that can represent its own invalid state

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 225965.
lawrence_danna added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

remove empty lines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69230

Files:
  lldb/scripts/Python/python-wrapper.swig
  llvm/include/llvm/ADT/Optional.h
  llvm/unittests/ADT/OptionalTest.cpp

Index: llvm/unittests/ADT/OptionalTest.cpp
===
--- llvm/unittests/ADT/OptionalTest.cpp
+++ llvm/unittests/ADT/OptionalTest.cpp
@@ -221,7 +221,7 @@
 TEST_F(OptionalTest, Emplace) {
   MultiArgConstructor::ResetCounts();
   Optional A;
-  
+
   A.emplace(1, 2);
   EXPECT_TRUE(A.hasValue());
   EXPECT_EQ(1, A->x);
@@ -592,3 +592,35 @@
 }
 
 } // end anonymous namespace
+
+class NaturalNumber {
+  friend struct llvm::optional_detail::has_is_valid;
+  int value;
+  constexpr NaturalNumber() : value(-1) {}
+
+public:
+  operator int() const { return value; }
+  NaturalNumber(int x) : value(x) { assert(value >= 0); }
+};
+
+template <> struct llvm::optional_detail::has_is_valid {
+  static const bool value = true;
+  static bool is_valid(const NaturalNumber &n) { return n.value >= 0; }
+  static constexpr NaturalNumber make_invalid() { return NaturalNumber(); }
+};
+
+TEST_F(OptionalTest, TestHasInvalid) {
+
+  constexpr Optional none;
+
+  static_assert(sizeof(Optional) == sizeof(int),
+"no extra bool");
+
+  Optional x = None;
+  EXPECT_FALSE(x.hasValue());
+  EXPECT_TRUE(x == none);
+  EXPECT_EQ(*reinterpret_cast(&x), -1);
+  x = NaturalNumber{42};
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 42);
+}
Index: llvm/include/llvm/ADT/Optional.h
===
--- llvm/include/llvm/ADT/Optional.h
+++ llvm/include/llvm/ADT/Optional.h
@@ -31,9 +31,12 @@
 
 struct in_place_t {};
 
+template  struct has_is_valid {
+  const static bool value = false;
+};
+
 /// Storage for any type.
-template ::value>
-class OptionalStorage {
+template  class OptionalStorage {
   union {
 char empty;
 T value;
@@ -138,7 +141,72 @@
   }
 };
 
-template  class OptionalStorage {
+template 
+class OptionalStorage<
+T, typename std::enable_if::value, void>::type> {
+
+  T value;
+  typedef has_is_valid THelper;
+
+public:
+  ~OptionalStorage() = default;
+
+  constexpr OptionalStorage() noexcept : value{THelper::make_invalid()} {}
+
+  OptionalStorage(OptionalStorage const &other) = default;
+  OptionalStorage(OptionalStorage &&other) = default;
+
+  OptionalStorage &operator=(OptionalStorage const &other) = default;
+  OptionalStorage &operator=(OptionalStorage &&other) = default;
+
+  template 
+  explicit OptionalStorage(in_place_t, Args &&... args)
+  : value(std::forward(args)...) {
+asert(THelper::is_valid(value));
+  }
+
+  void reset() noexcept { value = THelper::make_invalid; }
+
+  bool hasValue() const noexcept { return THelper::is_valid(value); }
+
+  T &getValue() LLVM_LVALUE_FUNCTION noexcept {
+assert(THelper::is_valid(value));
+return value;
+  }
+  T const &getValue() const LLVM_LVALUE_FUNCTION noexcept {
+assert(THelper::is_valid(value));
+return value;
+  }
+#if LLVM_HAS_RVALUE_REFERENCE_THIS
+  T &&getValue() && noexcept {
+assert(THelper::is_valid(value));
+return std::move(value);
+  }
+#endif
+
+  template  void emplace(Args &&... args) {
+value = T(std::forward(args)...);
+asert(THelper::is_valid(value));
+  }
+
+  OptionalStorage &operator=(T const &y) {
+assert(THelper::is_valid(y));
+value = y;
+return *this;
+  }
+
+  OptionalStorage &operator=(T &&y) {
+assert(THelper::is_valid(y));
+value = std::move(y);
+return *this;
+  }
+};
+
+template 
+class OptionalStorage<
+T, typename std::enable_if::value &&
+   !has_is_valid::value,
+   void>::type> {
   union {
 char empty;
 T value;
Index: lldb/scripts/Python/python-wrapper.swig
===
--- lldb/scripts/Python/python-wrapper.swig
+++ lldb/scripts/Python/python-wrapper.swig
@@ -48,7 +48,6 @@
 const lldb::BreakpointLocationSP& bp_loc_sp
 )
 {
-
 lldb::SBFrame sb_frame (frame_sp);
 lldb::SBBreakpointLocation sb_bp_loc(bp_loc_sp);
 
@@ -83,7 +82,6 @@
 const lldb::WatchpointSP& wp_sp
 )
 {
-
 lldb::SBFrame sb_frame (frame_sp);
 lldb::SBWatchpoint sb_wp(wp_sp);
 
@@ -118,7 +116,6 @@
 std::string& retval
 )
 {
-
 lldb::SBValue sb_value (valobj_sp);
 lldb::SBTypeSummaryOptions sb_options(options_sp.get());
 
@@ -190,8 +187,6 @@
 const lldb::ValueObjectSP& valobj_sp
 )
 {
-
-
 if (python_class_name == NULL || python_class_name[0] == '\0' || !session_dictionary_name)
 Py_RETURN_NONE;
 
@@ -228,8 +223,6 @@
 const lldb::DebuggerSP debugger_sp

[Lldb-commits] [lldb] e57fe85 - whitespace cleanup

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2019-10-21T22:48:27Z
New Revision: e57fe85a599a8cc4990d6f4605f86b89dcb952b3

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

LOG: whitespace cleanup

llvm-svn: 375465

Added: 


Modified: 
lldb/unittests/Expression/DWARFExpressionTest.cpp

Removed: 




diff  --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp 
b/lldb/unittests/Expression/DWARFExpressionTest.cpp
index f726d7766841..f6276de4e753 100644
--- a/lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -282,7 +282,7 @@ TEST(DWARFExpression, DW_OP_convert) {
   //
   // Positive tests.
   //
-  
+
   // Truncate to default unspecified (pointer-sized) type.
   EXPECT_THAT_EXPECTED(
   t.Eval({DW_OP_const8u, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, //



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

[Lldb-commits] [PATCH] D69214: remove multi-argument form of PythonObject::Reset()

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 225969.
lawrence_danna added a comment.

removed blank lines from python-wrapper.swig


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69214

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/scripts/Python/python-extensions.swig
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/Python/python-wrapper.swig
  lldb/scripts/lldb.swig
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonExceptionStateTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonExceptionStateTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonExceptionStateTests.cpp
+++ /dev/null
@@ -1,164 +0,0 @@
-//===-- PythonExceptionStateTest.cpp --*- C++
-//-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "gtest/gtest.h"
-
-#include "Plugins/ScriptInterpreter/Python/PythonDataObjects.h"
-#include "Plugins/ScriptInterpreter/Python/PythonExceptionState.h"
-#include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h"
-#include "Plugins/ScriptInterpreter/Python/lldb-python.h"
-
-#include "PythonTestSuite.h"
-
-using namespace lldb_private;
-
-class PythonExceptionStateTest : public PythonTestSuite {
-public:
-protected:
-  void RaiseException() {
-PyErr_SetString(PyExc_RuntimeError, "PythonExceptionStateTest test error");
-  }
-};
-
-TEST_F(PythonExceptionStateTest, TestExceptionStateChecking) {
-  PyErr_Clear();
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  RaiseException();
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestAcquisitionSemantics) {
-  PyErr_Clear();
-  PythonExceptionState no_error(false);
-  EXPECT_FALSE(no_error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-  RaiseException();
-  PythonExceptionState error(false);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  error.Discard();
-
-  PyErr_Clear();
-  RaiseException();
-  error.Acquire(false);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestDiscardSemantics) {
-  PyErr_Clear();
-
-  // Test that discarding an exception does not restore the exception
-  // state even when auto-restore==true is set
-  RaiseException();
-  PythonExceptionState error(true);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  error.Discard();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-}
-
-TEST_F(PythonExceptionStateTest, TestResetSemantics) {
-  PyErr_Clear();
-
-  // Resetting when auto-restore is true should restore.
-  RaiseException();
-  PythonExceptionState error(true);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  error.Reset();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-
-  // Resetting when auto-restore is false should discard.
-  RaiseException();
-  PythonExceptionState error2(false);
-  EXPECT_TRUE(error2.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  error2.Reset();
-  EXPECT_FALSE(error2.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestManualRestoreSemantics) {
-  PyErr_Clear();
-  RaiseException();
-  PythonExceptionState error(false);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  error.Restore();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestAutoRestoreSemantics) {
-  PyErr_Clear();
-  // Test that using the auto-restore flag correctly restores the exception
-  // state on destruction, and 

[Lldb-commits] [PATCH] D69230: RFC: specialized Optional for T that can represent its own invalid state

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 225967.
lawrence_danna added a comment.

ugh I edited the wrong revision, reverted.  Sorry for the noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69230

Files:
  llvm/include/llvm/ADT/Optional.h
  llvm/unittests/ADT/OptionalTest.cpp

Index: llvm/unittests/ADT/OptionalTest.cpp
===
--- llvm/unittests/ADT/OptionalTest.cpp
+++ llvm/unittests/ADT/OptionalTest.cpp
@@ -221,7 +221,7 @@
 TEST_F(OptionalTest, Emplace) {
   MultiArgConstructor::ResetCounts();
   Optional A;
-  
+
   A.emplace(1, 2);
   EXPECT_TRUE(A.hasValue());
   EXPECT_EQ(1, A->x);
@@ -592,3 +592,35 @@
 }
 
 } // end anonymous namespace
+
+class NaturalNumber {
+  friend struct llvm::optional_detail::has_is_valid;
+  int value;
+  constexpr NaturalNumber() : value(-1) {}
+
+public:
+  operator int() const { return value; }
+  NaturalNumber(int x) : value(x) { assert(value >= 0); }
+};
+
+template <> struct llvm::optional_detail::has_is_valid {
+  static const bool value = true;
+  static bool is_valid(const NaturalNumber &n) { return n.value >= 0; }
+  static constexpr NaturalNumber make_invalid() { return NaturalNumber(); }
+};
+
+TEST_F(OptionalTest, TestHasInvalid) {
+
+  constexpr Optional none;
+
+  static_assert(sizeof(Optional) == sizeof(int),
+"no extra bool");
+
+  Optional x = None;
+  EXPECT_FALSE(x.hasValue());
+  EXPECT_TRUE(x == none);
+  EXPECT_EQ(*reinterpret_cast(&x), -1);
+  x = NaturalNumber{42};
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 42);
+}
Index: llvm/include/llvm/ADT/Optional.h
===
--- llvm/include/llvm/ADT/Optional.h
+++ llvm/include/llvm/ADT/Optional.h
@@ -31,9 +31,12 @@
 
 struct in_place_t {};
 
+template  struct has_is_valid {
+  const static bool value = false;
+};
+
 /// Storage for any type.
-template ::value>
-class OptionalStorage {
+template  class OptionalStorage {
   union {
 char empty;
 T value;
@@ -138,7 +141,72 @@
   }
 };
 
-template  class OptionalStorage {
+template 
+class OptionalStorage<
+T, typename std::enable_if::value, void>::type> {
+
+  T value;
+  typedef has_is_valid THelper;
+
+public:
+  ~OptionalStorage() = default;
+
+  constexpr OptionalStorage() noexcept : value{THelper::make_invalid()} {}
+
+  OptionalStorage(OptionalStorage const &other) = default;
+  OptionalStorage(OptionalStorage &&other) = default;
+
+  OptionalStorage &operator=(OptionalStorage const &other) = default;
+  OptionalStorage &operator=(OptionalStorage &&other) = default;
+
+  template 
+  explicit OptionalStorage(in_place_t, Args &&... args)
+  : value(std::forward(args)...) {
+asert(THelper::is_valid(value));
+  }
+
+  void reset() noexcept { value = THelper::make_invalid; }
+
+  bool hasValue() const noexcept { return THelper::is_valid(value); }
+
+  T &getValue() LLVM_LVALUE_FUNCTION noexcept {
+assert(THelper::is_valid(value));
+return value;
+  }
+  T const &getValue() const LLVM_LVALUE_FUNCTION noexcept {
+assert(THelper::is_valid(value));
+return value;
+  }
+#if LLVM_HAS_RVALUE_REFERENCE_THIS
+  T &&getValue() && noexcept {
+assert(THelper::is_valid(value));
+return std::move(value);
+  }
+#endif
+
+  template  void emplace(Args &&... args) {
+value = T(std::forward(args)...);
+asert(THelper::is_valid(value));
+  }
+
+  OptionalStorage &operator=(T const &y) {
+assert(THelper::is_valid(y));
+value = y;
+return *this;
+  }
+
+  OptionalStorage &operator=(T &&y) {
+assert(THelper::is_valid(y));
+value = std::move(y);
+return *this;
+  }
+};
+
+template 
+class OptionalStorage<
+T, typename std::enable_if::value &&
+   !has_is_valid::value,
+   void>::type> {
   union {
 char empty;
 T value;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69285: minidump: Rename some architecture constants

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: amccarth, clayborg.
Herald added subscribers: fedor.sergeev, hiraditya, kristof.beyls.
Herald added projects: LLDB, LLVM.

The architecture enum contains two kinds of contstants: the "official" ones
defined by Microsoft, and unofficial constants added by breakpad to cover the
architectures not described by the first ones.

Up until now, there was no big need to differentiate between the two. However,
now that Microsoft has defined
https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/ns-sysinfoapi-system_info
a constant for ARM64, we have a name clash.

This patch renames all breakpad-defined constants with to include the prefix
"BP_". This frees up the name "ARM64", which I'll re-introduce with the new
"official" value in a follow-up patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69285

Files:
  lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
  llvm/include/llvm/BinaryFormat/MinidumpConstants.def
  llvm/lib/ObjectYAML/MinidumpYAML.cpp
  llvm/test/tools/obj2yaml/basic-minidump.yaml
  llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp

Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -33,7 +33,7 @@
 --- !minidump
 Streams:
   - Type:SystemInfo
-Processor Arch:  ARM64
+Processor Arch:  BP_ARM64
 Platform ID: Linux
 CPU:
   CPUID:   0x05060708
@@ -53,7 +53,7 @@
   auto ExpectedSysInfo = File.getSystemInfo();
   ASSERT_THAT_EXPECTED(ExpectedSysInfo, Succeeded());
   const SystemInfo &SysInfo = *ExpectedSysInfo;
-  EXPECT_EQ(ProcessorArchitecture::ARM64, SysInfo.ProcessorArch);
+  EXPECT_EQ(ProcessorArchitecture::BP_ARM64, SysInfo.ProcessorArch);
   EXPECT_EQ(OSPlatform::Linux, SysInfo.PlatformId);
   EXPECT_EQ(0x05060708u, SysInfo.CPU.Arm.CPUID);
 
Index: llvm/test/tools/obj2yaml/basic-minidump.yaml
===
--- llvm/test/tools/obj2yaml/basic-minidump.yaml
+++ llvm/test/tools/obj2yaml/basic-minidump.yaml
@@ -3,7 +3,7 @@
 --- !minidump
 Streams:
   - Type:SystemInfo
-Processor Arch:  ARM64
+Processor Arch:  BP_ARM64
 Platform ID: Linux
 CSD Version: Linux 3.13.0-91-generic
 CPU:
@@ -92,7 +92,7 @@
 # CHECK:  --- !minidump
 # CHECK-NEXT: Streams:
 # CHECK-NEXT:   - Type:SystemInfo
-# CHECK-NEXT: Processor Arch:  ARM64
+# CHECK-NEXT: Processor Arch:  BP_ARM64
 # CHECK-NEXT: Platform ID: Linux
 # CHECK-NEXT: CSD Version: Linux 3.13.0-91-generic
 # CHECK-NEXT: CPU:
Index: llvm/lib/ObjectYAML/MinidumpYAML.cpp
===
--- llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -336,7 +336,7 @@
 IO.mapOptional("CPU", Info.CPU.X86);
 break;
   case ProcessorArchitecture::ARM:
-  case ProcessorArchitecture::ARM64:
+  case ProcessorArchitecture::BP_ARM64:
 IO.mapOptional("CPU", Info.CPU.Arm);
 break;
   default:
Index: llvm/include/llvm/BinaryFormat/MinidumpConstants.def
===
--- llvm/include/llvm/BinaryFormat/MinidumpConstants.def
+++ llvm/include/llvm/BinaryFormat/MinidumpConstants.def
@@ -85,21 +85,21 @@
 HANDLE_MDMP_STREAM_TYPE(0xFACEDEAD, FacebookAbortReason)
 HANDLE_MDMP_STREAM_TYPE(0xFACEE000, FacebookThreadName)
 
-HANDLE_MDMP_ARCH(0x, X86)  // PROCESSOR_ARCHITECTURE_INTEL
-HANDLE_MDMP_ARCH(0x0001, MIPS) // PROCESSOR_ARCHITECTURE_MIPS
-HANDLE_MDMP_ARCH(0x0002, Alpha)// PROCESSOR_ARCHITECTURE_ALPHA
-HANDLE_MDMP_ARCH(0x0003, PPC)  // PROCESSOR_ARCHITECTURE_PPC
-HANDLE_MDMP_ARCH(0x0004, SHX)  // PROCESSOR_ARCHITECTURE_SHX (Super-H)
-HANDLE_MDMP_ARCH(0x0005, ARM)  // PROCESSOR_ARCHITECTURE_ARM
-HANDLE_MDMP_ARCH(0x0006, IA64) // PROCESSOR_ARCHITECTURE_IA64
-HANDLE_MDMP_ARCH(0x0007, Alpha64)  // PROCESSOR_ARCHITECTURE_ALPHA64
-HANDLE_MDMP_ARCH(0x0008, MSIL) // PROCESSOR_ARCHITECTURE_MSIL
-HANDLE_MDMP_ARCH(0x0009, AMD64)// PROCESSOR_ARCHITECTURE_AMD64
-HANDLE_MDMP_ARCH(0x000a, X86Win64) // PROCESSOR_ARCHITECTURE_IA32_ON_WIN64
-HANDLE_MDMP_ARCH(0x8001, SPARC)// Breakpad-defined value for SPARC
-HANDLE_MDMP_ARCH(0x8002, PPC64)// Breakpad-defined value for PPC64
-HANDLE_MDMP_ARCH(0x8003, ARM64)// Breakpad-defined value for ARM64
-HANDLE_MDMP_ARCH(0x8004, MIPS64)   // Breakpad-defined value for MIPS64
+HANDLE_MDMP_ARCH(0x, X86)   // PROCESSOR_ARCHITECTURE_INTEL
+HANDLE_MDMP_ARCH(0x0001, MIPS)  // PROCESSOR_ARCHITECTURE_MIPS
+HANDLE_MDMP_ARCH(0x0002, Alpha) // PROCESSOR_ARCHITECTURE_ALPHA
+HANDLE_MDMP_ARCH(0x0003, PPC)   // PROCESSOR_ARCHITECTURE_PPC
+HANDLE_MDMP_ARCH(0x0004, SHX)   // PROCESSOR_ARCHITECTURE_SHX (Super-H)
+HANDLE_MDMP_ARCH(0

[Lldb-commits] [PATCH] D69286: I implemented the features listed in this document: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0616r0.pdf and built libc++ using ninja without any errors/w

[Fady Farag via Phabricator via lldb-commits]

Afadyfarag created this revision.
Afadyfarag added reviewers: clayborg, aprantl, JDevlieghere.
Herald added subscribers: libcxx-commits, ldionne, christof, mgorny.
Herald added a reviewer: EricWF.
Herald added a project: libc++.

1. Changes:

  libcxx/include/numeric


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69286

Files:
  libcxx/CMakeLists.txt
  libcxx/include/numeric
  libcxx/include/set

Index: libcxx/include/set
===
--- libcxx/include/set
+++ libcxx/include/set
@@ -472,13 +472,13 @@
 
 #if _LIBCPP_STD_VER > 14
 typedef __set_node_handle node_type;
-typedef __insert_return_type insert_return_type;
+typedef __insert_return_type insert_return_type
 #endif
 
 template 
 friend class _LIBCPP_TEMPLATE_VIS set;
 template 
-friend class _LIBCPP_TEMPLATE_VIS multiset;
+friend class _LIBCPP_TEMPLATE_VIS multiset
 
 _LIBCPP_INLINE_VISIBILITY
 set()
@@ -486,7 +486,7 @@
 is_nothrow_default_constructible::value &&
 is_nothrow_default_constructible::value &&
 is_nothrow_copy_constructible::value)
-: __tree_(value_compare()) {}
+: __tree_(value_compare()) {
 
 _LIBCPP_INLINE_VISIBILITY
 explicit set(const value_compare& __comp)
Index: libcxx/include/numeric
===
--- libcxx/include/numeric
+++ libcxx/include/numeric
@@ -143,7 +143,9 @@
 
 #include <__config>
 #include 
+#if _LIBCPP_STD_VER > 17 
 #include  // for numeric_limits
+#endif
 #include 
 #include  // for isnormal
 #include 
@@ -172,8 +174,13 @@
 _Tp
 accumulate(_InputIterator __first, _InputIterator __last, _Tp __init, _BinaryOperation __binary_op)
 {
-for (; __first != __last; ++__first)
+for (; __first != __last; ++__first) {
+#if _LIBCPP_STD_VER > 17
+__init = __binary_op(_VSTD::move(__init), *__first);
+#else
 __init = __binary_op(__init, *__first);
+#endif
+}
 return __init;
 }
 
@@ -222,8 +229,13 @@
 inner_product(_InputIterator1 __first1, _InputIterator1 __last1, _InputIterator2 __first2,
   _Tp __init, _BinaryOperation1 __binary_op1, _BinaryOperation2 __binary_op2)
 {
-for (; __first1 != __last1; ++__first1, (void) ++__first2)
+for (; __first1 != __last1; ++__first1, (void) ++__first2) {
+#if _LIBCPP_STD_VER > 17
+__init = __binary_op1(_VSTD::move(__init), __binary_op2(*__first1, *__first2));
+#else
 __init = __binary_op1(__init, __binary_op2(*__first1, *__first2));
+#endif
+}
 return __init;
 }
 
@@ -292,8 +304,13 @@
 *__result = __t;
 for (++__first, (void) ++__result; __first != __last; ++__first, (void) ++__result)
 {
+#if _LIBCPP_STD_VER > 17
+__t = __binary_op(_VSTD::move(__t), *__first);
+*__result = __t;
+#else
 __t = __binary_op(__t, *__first);
 *__result = __t;
+#endif
 }
 }
 return __result;
@@ -442,7 +459,11 @@
 for (++__first, (void) ++__result; __first != __last; ++__first, (void) ++__result)
 {
 typename iterator_traits<_InputIterator>::value_type __t2(*__first);
+#if _LIBCPP_STD_VER > 17
+*__result = __binary_op(__t2, _VSTD::move(__t1));
+#else
 *__result = __binary_op(__t2, __t1);
+#endif
 __t1 = _VSTD::move(__t2);
 }
 }
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -533,6 +533,16 @@
 # Thus, we do nothing and hope we don't accidentally include any of the C++
 # headers
 add_compile_flags_if_supported(-nostdinc++)
+add_compile_flags_if_supported(-fcolor-diagnostics)
+
+option (FORCE_COLORED_OUTPUT "Always produce ANSI-colored output (GNU/Clang only)." FALSE)
+if (${FORCE_COLORED_OUTPUT})
+if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
+   add_compile_options (-fdiagnostics-color=always)
+elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
+   add_compile_options (-fcolor-diagnostics)
+endif ()
+endif ()
 
 # Hide all inline function definitions which have not explicitly been marked
 # visible. This prevents new definitions for inline functions from appearing in
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D69148#1717361 , @vsk wrote:

> A problem I'm encountering with this is that the static initializer from 
> Signals.inc don't appear to be re-run between death tests. This makes the 
> death tests pretty brittle, imo, as swapping the order of the tests can break 
> them. Do you think the extra coverage is worth it? (See 
> https://reviews.llvm.org/D69283 for what this looks like.)


Yeah, not being able to run each test in a separate process is a big problem 
here. I'm not too worried about adding those tests here. However, looking at 
those tests, and your inline comment, I get the impression you're not 
understanding how the various signal functions actually work. One definitely 
shouldn't be calling the `signal` function manually, and also using the llvm 
signal handler functions (at least, not without a very big comment explaining 
what he's doing). They both do the same thing -- set the global signal handler. 
And since the llvm handling is initialized lazily this sets us up for all sorts 
of unpredictible and confusing behavior. See the inline comment for details.




Comment at: lldb/tools/driver/Driver.cpp:850
 #if !defined(_MSC_VER)
   signal(SIGPIPE, SIG_IGN);
   signal(SIGWINCH, sigwinch_handler);

vsk wrote:
> labath wrote:
> > I don't think this line does anything anymore.
> I don't think that's true. LLVM still does `registerHandler(SIGPIPE, 
> SignalKind::IsKill)` in `RegisterHandlers`. At least, removing the 
> `signal(SIGPIPE, SIG_IGN)` calls causes the unit test to "fail" (i.e. the 
> test RUNs but doesn't reach OK).
Ah, right, I see now. This does have a very subtle change in behavior -- it 
changes the signal handler that llvm restores *after* the signal has been 
received the first time (Signals.inc:355). Normally, it would put SIG_DFL there 
(which would kill the process). But with this, it puts SIG_IGN there. So this 
changes what we do when we receive SIGPIPE for the *second* time.

This means that the first SIGPIPE signal is ignored by the virtue of us calling 
`SetPipeSignalFunction(nullptr)`, but all subsequent SIGPIPEs are ignored 
thanks to this SIG_IGN. It also means this whole talk of "being able to survive 
multiple SIGPIPEs" is moot, as llvm does not allow that. In fact it looks like 
the very first occurence of SIGPIPE will cause _all_ signal handlers to be 
uninstalled. I think this is a very confusing situation to be in.

If we don't want to make llvm support handling/ignoring multiple SIGPIPEs 
correctly, then I think that a much better solution would be to just work 
around this in lldb by making sure the llvm handler is never invoked for 
SIGPIPE (this can be arranged by making sure `signal(SIGPIPE, SIG_IGN)` is 
called *after* llvm installs its handlers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69148



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

[Lldb-commits] [PATCH] D69230: RFC: specialized Optional for T that can represent its own invalid state

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 225975.
lawrence_danna added a comment.

fix some silly bugs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69230

Files:
  llvm/include/llvm/ADT/Optional.h
  llvm/unittests/ADT/OptionalTest.cpp

Index: llvm/unittests/ADT/OptionalTest.cpp
===
--- llvm/unittests/ADT/OptionalTest.cpp
+++ llvm/unittests/ADT/OptionalTest.cpp
@@ -221,7 +221,7 @@
 TEST_F(OptionalTest, Emplace) {
   MultiArgConstructor::ResetCounts();
   Optional A;
-  
+
   A.emplace(1, 2);
   EXPECT_TRUE(A.hasValue());
   EXPECT_EQ(1, A->x);
@@ -592,3 +592,50 @@
 }
 
 } // end anonymous namespace
+
+class NaturalNumber {
+  friend struct llvm::optional_detail::has_is_valid;
+  int value;
+  constexpr NaturalNumber() : value(-1) {}
+
+public:
+  operator int() const { return value; }
+  NaturalNumber(int x) : value(x) { assert(value >= 0); }
+};
+
+template <> struct llvm::optional_detail::has_is_valid {
+  static const bool value = true;
+  static bool is_valid(const NaturalNumber &n) { return n.value >= 0; }
+  static constexpr NaturalNumber make_invalid() { return NaturalNumber(); }
+};
+
+TEST_F(OptionalTest, TestHasInvalid) {
+
+  constexpr Optional none;
+
+  static_assert(sizeof(Optional) == sizeof(int),
+"no extra bool");
+
+  Optional x = None;
+  EXPECT_FALSE(x.hasValue());
+  EXPECT_TRUE(x == none);
+  EXPECT_EQ(*reinterpret_cast(&x), -1);
+  x = NaturalNumber{42};
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 42);
+  x.reset();
+  EXPECT_FALSE(x.hasValue());
+
+  x.emplace(1234);
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 1234);
+
+  x = Optional(4321);
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 4321);
+
+  auto y = Optional(0x0dedbeef);
+  x = y;
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), (int)0x0dedbeef);
+}
Index: llvm/include/llvm/ADT/Optional.h
===
--- llvm/include/llvm/ADT/Optional.h
+++ llvm/include/llvm/ADT/Optional.h
@@ -31,9 +31,12 @@
 
 struct in_place_t {};
 
+template  struct has_is_valid {
+  const static bool value = false;
+};
+
 /// Storage for any type.
-template ::value>
-class OptionalStorage {
+template  class OptionalStorage {
   union {
 char empty;
 T value;
@@ -138,7 +141,72 @@
   }
 };
 
-template  class OptionalStorage {
+template 
+class OptionalStorage<
+T, typename std::enable_if::value, void>::type> {
+
+  T value;
+  typedef has_is_valid THelper;
+
+public:
+  ~OptionalStorage() = default;
+
+  constexpr OptionalStorage() noexcept : value{THelper::make_invalid()} {}
+
+  OptionalStorage(OptionalStorage const &other) = default;
+  OptionalStorage(OptionalStorage &&other) = default;
+
+  OptionalStorage &operator=(OptionalStorage const &other) = default;
+  OptionalStorage &operator=(OptionalStorage &&other) = default;
+
+  template 
+  explicit OptionalStorage(in_place_t, Args &&... args)
+  : value(std::forward(args)...) {
+assert(THelper::is_valid(value));
+  }
+
+  void reset() noexcept { value = THelper::make_invalid(); }
+
+  bool hasValue() const noexcept { return THelper::is_valid(value); }
+
+  T &getValue() LLVM_LVALUE_FUNCTION noexcept {
+assert(THelper::is_valid(value));
+return value;
+  }
+  T const &getValue() const LLVM_LVALUE_FUNCTION noexcept {
+assert(THelper::is_valid(value));
+return value;
+  }
+#if LLVM_HAS_RVALUE_REFERENCE_THIS
+  T &&getValue() && noexcept {
+assert(THelper::is_valid(value));
+return std::move(value);
+  }
+#endif
+
+  template  void emplace(Args &&... args) {
+value = T(std::forward(args)...);
+assert(THelper::is_valid(value));
+  }
+
+  OptionalStorage &operator=(T const &y) {
+assert(THelper::is_valid(y));
+value = y;
+return *this;
+  }
+
+  OptionalStorage &operator=(T &&y) {
+assert(THelper::is_valid(y));
+value = std::move(y);
+return *this;
+  }
+};
+
+template 
+class OptionalStorage<
+T, typename std::enable_if::value &&
+   !has_is_valid::value,
+   void>::type> {
   union {
 char empty;
 T value;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D69148#1717453 , @labath wrote:

> I'm not too worried about adding those tests here.


I'm not too worried about *not* adding those tests here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69148



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

[Lldb-commits] [PATCH] D69273: ValueObject: Fix a crash related to children address type computation

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Except for the comment comment this looks fine.

I think the Host -> Load address code isn't quite right, though it looks like 
that's not your doing.

The main reason why you would copy a ValueObject into Host memory it is to 
freeze-dry it as a ConstResult.  Since that is supposed to represent the object 
at that point in time, it should only be valid to you ask the ValueObject to 
produce its children if the process is at the same StopID as when the 
ValueObject was made.  Then the ValueObject system should fetch that more data 
and include it in the freeze-dried object.  But if the StopID has moved on, you 
should just give an error: "Can't travel back in time to fetch that extra data".

At the ValueObject level, however, we only know how the data is stored (in Host 
or Process) and not why.  So it's harder to get this behavior right.

As I said, however, I don't think this planned design was ever carried out 
successfully, so I don't think this will have broken anything.




Comment at: source/Core/ValueObject.cpp:162-163
+// of the "next" member of LinkedListNode will become load addresses if
+// we have a live process, or remain what a file address if it what a
+// file address.
+if (process_is_alive && is_pointer_or_ref)

"remain a file address if it was a file address."?


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

https://reviews.llvm.org/D69273



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

[Lldb-commits] [PATCH] D69230: RFC: specialized Optional for T that can represent its own invalid state

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 225980.
lawrence_danna added a comment.

call the traits struct OptionalInfo for consistency with DenseMap


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69230

Files:
  llvm/include/llvm/ADT/Optional.h
  llvm/unittests/ADT/OptionalTest.cpp

Index: llvm/unittests/ADT/OptionalTest.cpp
===
--- llvm/unittests/ADT/OptionalTest.cpp
+++ llvm/unittests/ADT/OptionalTest.cpp
@@ -221,7 +221,7 @@
 TEST_F(OptionalTest, Emplace) {
   MultiArgConstructor::ResetCounts();
   Optional A;
-  
+
   A.emplace(1, 2);
   EXPECT_TRUE(A.hasValue());
   EXPECT_EQ(1, A->x);
@@ -592,3 +592,57 @@
 }
 
 } // end anonymous namespace
+
+class NaturalNumber {
+  friend struct llvm::OptionalInfo;
+  int value;
+  constexpr NaturalNumber() : value(-1) {}
+
+public:
+  operator int() const { return value; }
+  NaturalNumber(int x) : value(x) { assert(value >= 0); }
+};
+
+template <> struct llvm::OptionalInfo {
+  static const bool has_is_valid = true;
+  static bool is_valid(const NaturalNumber &n) { return n.value >= 0; }
+  static constexpr NaturalNumber make_invalid() { return NaturalNumber(); }
+};
+
+static_assert(Optional::storage_type::uses_trivial_copy,
+  "check Optional");
+static_assert(Optional::storage_type::uses_is_valid,
+  "check Optional");
+static_assert(Optional>::storage_type::is_generic_storage,
+  "check Optional");
+
+TEST_F(OptionalTest, TestHasInvalid) {
+
+  constexpr Optional none;
+
+  static_assert(sizeof(Optional) == sizeof(int),
+"no extra bool");
+
+  Optional x = None;
+  EXPECT_FALSE(x.hasValue());
+  EXPECT_TRUE(x == none);
+  EXPECT_EQ(*reinterpret_cast(&x), -1);
+  x = NaturalNumber{42};
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 42);
+  x.reset();
+  EXPECT_FALSE(x.hasValue());
+
+  x.emplace(1234);
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 1234);
+
+  x = Optional(4321);
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 4321);
+
+  auto y = Optional(0x0dedbeef);
+  x = y;
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), (int)0x0dedbeef);
+}
Index: llvm/include/llvm/ADT/Optional.h
===
--- llvm/include/llvm/ADT/Optional.h
+++ llvm/include/llvm/ADT/Optional.h
@@ -27,12 +27,19 @@
 
 class raw_ostream;
 
+template  struct OptionalInfo {
+  typedef T type;
+  static const bool has_is_valid = false;
+  static const bool is_trivially_copyable =
+  std::is_trivially_copyable::value;
+};
+
 namespace optional_detail {
 
 struct in_place_t {};
 
 /// Storage for any type.
-template ::value>
+template , typename Enable = void>
 class OptionalStorage {
   union {
 char empty;
@@ -41,6 +48,8 @@
   bool hasVal;
 
 public:
+  static const bool is_generic_storage = true;
+
   ~OptionalStorage() { reset(); }
 
   OptionalStorage() noexcept : empty(), hasVal(false) {}
@@ -138,7 +147,77 @@
   }
 };
 
-template  class OptionalStorage {
+template 
+class OptionalStorage<
+T, OptionalInfo,
+typename std::enable_if::has_is_valid, void>::type> {
+
+  T value;
+  typedef OptionalInfo Info;
+
+public:
+  static const bool uses_is_valid = true;
+
+  ~OptionalStorage() = default;
+
+  constexpr OptionalStorage() noexcept : value{Info::make_invalid()} {}
+
+  OptionalStorage(OptionalStorage const &other) = default;
+  OptionalStorage(OptionalStorage &&other) = default;
+
+  OptionalStorage &operator=(OptionalStorage const &other) = default;
+  OptionalStorage &operator=(OptionalStorage &&other) = default;
+
+  template 
+  explicit OptionalStorage(in_place_t, Args &&... args)
+  : value(std::forward(args)...) {
+assert(Info::is_valid(value));
+  }
+
+  void reset() noexcept { value = Info::make_invalid(); }
+
+  bool hasValue() const noexcept { return Info::is_valid(value); }
+
+  T &getValue() LLVM_LVALUE_FUNCTION noexcept {
+assert(Info::is_valid(value));
+return value;
+  }
+  T const &getValue() const LLVM_LVALUE_FUNCTION noexcept {
+assert(Info::is_valid(value));
+return value;
+  }
+#if LLVM_HAS_RVALUE_REFERENCE_THIS
+  T &&getValue() && noexcept {
+assert(Info::is_valid(value));
+return std::move(value);
+  }
+#endif
+
+  template  void emplace(Args &&... args) {
+value = T(std::forward(args)...);
+assert(Info::is_valid(value));
+  }
+
+  OptionalStorage &operator=(T const &y) {
+assert(Info::is_valid(y));
+value = y;
+return *this;
+  }
+
+  OptionalStorage &operator=(T &&y) {
+assert(Info::is_valid(y));
+value = std::move(y);
+return *this;
+  }
+};
+
+template 
+class OptionalStorage<
+T, OptionalInfo,
+typename std::enable_if::is_trivially_copyable &&
+!OptionalInfo::has_is_valid,
+void>::type> {
+
   union

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

[Vedant Kumar via Phabricator via lldb-commits]

vsk marked an inline comment as done.
vsk added a comment.

I admit I hadn't paid enough attention to the order in which signal handlers 
were registered in lldb.




Comment at: lldb/tools/driver/Driver.cpp:850
 #if !defined(_MSC_VER)
   signal(SIGPIPE, SIG_IGN);
   signal(SIGWINCH, sigwinch_handler);

labath wrote:
> vsk wrote:
> > labath wrote:
> > > I don't think this line does anything anymore.
> > I don't think that's true. LLVM still does `registerHandler(SIGPIPE, 
> > SignalKind::IsKill)` in `RegisterHandlers`. At least, removing the 
> > `signal(SIGPIPE, SIG_IGN)` calls causes the unit test to "fail" (i.e. the 
> > test RUNs but doesn't reach OK).
> Ah, right, I see now. This does have a very subtle change in behavior -- it 
> changes the signal handler that llvm restores *after* the signal has been 
> received the first time (Signals.inc:355). Normally, it would put SIG_DFL 
> there (which would kill the process). But with this, it puts SIG_IGN there. 
> So this changes what we do when we receive SIGPIPE for the *second* time.
> 
> This means that the first SIGPIPE signal is ignored by the virtue of us 
> calling `SetPipeSignalFunction(nullptr)`, but all subsequent SIGPIPEs are 
> ignored thanks to this SIG_IGN. It also means this whole talk of "being able 
> to survive multiple SIGPIPEs" is moot, as llvm does not allow that. In fact 
> it looks like the very first occurence of SIGPIPE will cause _all_ signal 
> handlers to be uninstalled. I think this is a very confusing situation to be 
> in.
> 
> If we don't want to make llvm support handling/ignoring multiple SIGPIPEs 
> correctly, then I think that a much better solution would be to just work 
> around this in lldb by making sure the llvm handler is never invoked for 
> SIGPIPE (this can be arranged by making sure `signal(SIGPIPE, SIG_IGN)` is 
> called *after* llvm installs its handlers.
Point taken. I agree that when a SIGPIPE is received, it doesn't make sense to 
reset the signal handler state as it was prior to llvm's handler registration. 
Do you think it'd be reasonable to:

- In lldb, force llvm to register its handlers, then immediately ignore SIGPIPE.
- Revert the `SetPipeSignalFunction` changes, as they would no longer be 
required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69148



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

[Lldb-commits] [PATCH] D69230: RFC: specialized Optional for T that can represent its own invalid state

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

In D69230#1717364 , @labath wrote:

>




> The thing I would like to see here is to have this behaviour be configurable 
> via a traits argument of the Optional class, similarly to how DenseMap allows 
> the type to specify default traits by specialising DenseMapInfo (I recommend 
> taking a look at that for inspiration/consistency),

So that would mean actually adding the Info class as a template parameter to 
Optional, right?   It looks like that would be quite disruptive, as there's 
other headers that predeclare Optional as taking only a single template 
parameter.

I'm not sure I understand what the advantage of having that extra level of 
customization would be.Are there situations where `OptionalInfo`  is 
specialized  in `Foo.h`, but some other code is going to want to use its own 
Info struct to customize `Foo` optionals?I can see how you'd want that for 
maps, because different code has different requirements for their maps, even 
with the same key types.   But it seems like for Optionals there's probably one 
right way to make an `Optional` for each `T`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69230



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

[Lldb-commits] [PATCH] D69230: RFC: specialized Optional for T that can represent its own invalid state

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 225984.
lawrence_danna added a comment.

edit comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69230

Files:
  llvm/include/llvm/ADT/Optional.h
  llvm/unittests/ADT/OptionalTest.cpp

Index: llvm/unittests/ADT/OptionalTest.cpp
===
--- llvm/unittests/ADT/OptionalTest.cpp
+++ llvm/unittests/ADT/OptionalTest.cpp
@@ -221,7 +221,7 @@
 TEST_F(OptionalTest, Emplace) {
   MultiArgConstructor::ResetCounts();
   Optional A;
-  
+
   A.emplace(1, 2);
   EXPECT_TRUE(A.hasValue());
   EXPECT_EQ(1, A->x);
@@ -592,3 +592,57 @@
 }
 
 } // end anonymous namespace
+
+class NaturalNumber {
+  friend struct llvm::OptionalInfo;
+  int value;
+  constexpr NaturalNumber() : value(-1) {}
+
+public:
+  operator int() const { return value; }
+  NaturalNumber(int x) : value(x) { assert(value >= 0); }
+};
+
+template <> struct llvm::OptionalInfo {
+  static const bool has_is_valid = true;
+  static bool is_valid(const NaturalNumber &n) { return n.value >= 0; }
+  static constexpr NaturalNumber make_invalid() { return NaturalNumber(); }
+};
+
+static_assert(Optional::storage_type::uses_trivial_copy,
+  "check Optional");
+static_assert(Optional::storage_type::uses_is_valid,
+  "check Optional");
+static_assert(Optional>::storage_type::is_generic_storage,
+  "check Optional");
+
+TEST_F(OptionalTest, TestHasInvalid) {
+
+  constexpr Optional none;
+
+  static_assert(sizeof(Optional) == sizeof(int),
+"no extra bool");
+
+  Optional x = None;
+  EXPECT_FALSE(x.hasValue());
+  EXPECT_TRUE(x == none);
+  EXPECT_EQ(*reinterpret_cast(&x), -1);
+  x = NaturalNumber{42};
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 42);
+  x.reset();
+  EXPECT_FALSE(x.hasValue());
+
+  x.emplace(1234);
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 1234);
+
+  x = Optional(4321);
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 4321);
+
+  auto y = Optional(0x0dedbeef);
+  x = y;
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), (int)0x0dedbeef);
+}
Index: llvm/include/llvm/ADT/Optional.h
===
--- llvm/include/llvm/ADT/Optional.h
+++ llvm/include/llvm/ADT/Optional.h
@@ -27,12 +27,33 @@
 
 class raw_ostream;
 
+// This template may be specialized in order to customize
+// how Optional is implemented.If you do specialize this,
+// you must do so in the same header that defines T.   Otherwise
+// there could be some translation units that see your specialized
+// OptionalInfo and other translation units that do not.
+//
+// The only available customization is to set has_is_valid = true,
+// and implement is_valid() and make_invalid().
+// This will cause Optional to be represented as: class { T value; }
+// None will be represented by an invalid T, as returned by
+// make_invalid().   T's representation of invalid values should
+// be private and only shared with OptionalInfo
+template  struct OptionalInfo {
+  typedef T type;
+  static const bool has_is_valid = false;
+  static const bool is_trivially_copyable =
+  std::is_trivially_copyable::value;
+  // static bool is_valid(const T &n);
+  // static constexpr T make_invalid();
+};
+
 namespace optional_detail {
 
 struct in_place_t {};
 
 /// Storage for any type.
-template ::value>
+template , typename Enable = void>
 class OptionalStorage {
   union {
 char empty;
@@ -41,6 +62,8 @@
   bool hasVal;
 
 public:
+  static const bool is_generic_storage = true;
+
   ~OptionalStorage() { reset(); }
 
   OptionalStorage() noexcept : empty(), hasVal(false) {}
@@ -138,7 +161,77 @@
   }
 };
 
-template  class OptionalStorage {
+template 
+class OptionalStorage<
+T, OptionalInfo,
+typename std::enable_if::has_is_valid, void>::type> {
+
+  T value;
+  typedef OptionalInfo Info;
+
+public:
+  static const bool uses_is_valid = true;
+
+  ~OptionalStorage() = default;
+
+  constexpr OptionalStorage() noexcept : value{Info::make_invalid()} {}
+
+  OptionalStorage(OptionalStorage const &other) = default;
+  OptionalStorage(OptionalStorage &&other) = default;
+
+  OptionalStorage &operator=(OptionalStorage const &other) = default;
+  OptionalStorage &operator=(OptionalStorage &&other) = default;
+
+  template 
+  explicit OptionalStorage(in_place_t, Args &&... args)
+  : value(std::forward(args)...) {
+assert(Info::is_valid(value));
+  }
+
+  void reset() noexcept { value = Info::make_invalid(); }
+
+  bool hasValue() const noexcept { return Info::is_valid(value); }
+
+  T &getValue() LLVM_LVALUE_FUNCTION noexcept {
+assert(Info::is_valid(value));
+return value;
+  }
+  T const &getValue() const LLVM_LVALUE_FUNCTION noexcept {
+assert(Info::is_valid(value));
+return value;
+  }
+#if LLVM_HAS_RVALUE_REFERENCE_THIS
+  T 

[Lldb-commits] [PATCH] D69230: RFC: specialized Optional for T that can represent its own invalid state

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 225983.
lawrence_danna added a comment.

add a comment explaining OptionalInfo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69230

Files:
  llvm/include/llvm/ADT/Optional.h
  llvm/unittests/ADT/OptionalTest.cpp

Index: llvm/unittests/ADT/OptionalTest.cpp
===
--- llvm/unittests/ADT/OptionalTest.cpp
+++ llvm/unittests/ADT/OptionalTest.cpp
@@ -221,7 +221,7 @@
 TEST_F(OptionalTest, Emplace) {
   MultiArgConstructor::ResetCounts();
   Optional A;
-  
+
   A.emplace(1, 2);
   EXPECT_TRUE(A.hasValue());
   EXPECT_EQ(1, A->x);
@@ -592,3 +592,57 @@
 }
 
 } // end anonymous namespace
+
+class NaturalNumber {
+  friend struct llvm::OptionalInfo;
+  int value;
+  constexpr NaturalNumber() : value(-1) {}
+
+public:
+  operator int() const { return value; }
+  NaturalNumber(int x) : value(x) { assert(value >= 0); }
+};
+
+template <> struct llvm::OptionalInfo {
+  static const bool has_is_valid = true;
+  static bool is_valid(const NaturalNumber &n) { return n.value >= 0; }
+  static constexpr NaturalNumber make_invalid() { return NaturalNumber(); }
+};
+
+static_assert(Optional::storage_type::uses_trivial_copy,
+  "check Optional");
+static_assert(Optional::storage_type::uses_is_valid,
+  "check Optional");
+static_assert(Optional>::storage_type::is_generic_storage,
+  "check Optional");
+
+TEST_F(OptionalTest, TestHasInvalid) {
+
+  constexpr Optional none;
+
+  static_assert(sizeof(Optional) == sizeof(int),
+"no extra bool");
+
+  Optional x = None;
+  EXPECT_FALSE(x.hasValue());
+  EXPECT_TRUE(x == none);
+  EXPECT_EQ(*reinterpret_cast(&x), -1);
+  x = NaturalNumber{42};
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 42);
+  x.reset();
+  EXPECT_FALSE(x.hasValue());
+
+  x.emplace(1234);
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 1234);
+
+  x = Optional(4321);
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), 4321);
+
+  auto y = Optional(0x0dedbeef);
+  x = y;
+  EXPECT_TRUE(x.hasValue());
+  EXPECT_EQ((int)x.getValue(), (int)0x0dedbeef);
+}
Index: llvm/include/llvm/ADT/Optional.h
===
--- llvm/include/llvm/ADT/Optional.h
+++ llvm/include/llvm/ADT/Optional.h
@@ -27,12 +27,33 @@
 
 class raw_ostream;
 
+// This template may be specialized in order to customize
+// how Optional is implemented.If you do specialize this,
+// you must do so in the same header that defines T.   Otherwise
+// there could be some translation units that see your specialized
+// OptionalInfo and other translation units that do not.
+//
+// The only available customization is to set has_is_valid = true,
+// and implement is_valid() and make_invalid().
+// This will cause Optional to be represented as: class { T value; }
+// None will be represented by an invalid T, as returned by
+// make_invalid().   T's representation of invalid values should
+// be private and only shared with OptionalInfo
+template  struct OptionalInfo {
+  typedef T type;
+  static const bool has_is_valid = false;
+  static const bool is_trivially_copyable =
+  std::is_trivially_copyable::value;
+  // static bool is_valid(const T &n) { return n.value >= 0; }
+  // static constexpr T make_invalid() { return T(); }
+};
+
 namespace optional_detail {
 
 struct in_place_t {};
 
 /// Storage for any type.
-template ::value>
+template , typename Enable = void>
 class OptionalStorage {
   union {
 char empty;
@@ -41,6 +62,8 @@
   bool hasVal;
 
 public:
+  static const bool is_generic_storage = true;
+
   ~OptionalStorage() { reset(); }
 
   OptionalStorage() noexcept : empty(), hasVal(false) {}
@@ -138,7 +161,77 @@
   }
 };
 
-template  class OptionalStorage {
+template 
+class OptionalStorage<
+T, OptionalInfo,
+typename std::enable_if::has_is_valid, void>::type> {
+
+  T value;
+  typedef OptionalInfo Info;
+
+public:
+  static const bool uses_is_valid = true;
+
+  ~OptionalStorage() = default;
+
+  constexpr OptionalStorage() noexcept : value{Info::make_invalid()} {}
+
+  OptionalStorage(OptionalStorage const &other) = default;
+  OptionalStorage(OptionalStorage &&other) = default;
+
+  OptionalStorage &operator=(OptionalStorage const &other) = default;
+  OptionalStorage &operator=(OptionalStorage &&other) = default;
+
+  template 
+  explicit OptionalStorage(in_place_t, Args &&... args)
+  : value(std::forward(args)...) {
+assert(Info::is_valid(value));
+  }
+
+  void reset() noexcept { value = Info::make_invalid(); }
+
+  bool hasValue() const noexcept { return Info::is_valid(value); }
+
+  T &getValue() LLVM_LVALUE_FUNCTION noexcept {
+assert(Info::is_valid(value));
+return value;
+  }
+  T const &getValue() const LLVM_LVALUE_FUNCTION noexcept {
+assert(Info::is_valid(value));
+

[Lldb-commits] [PATCH] D68856: convert SBDebugger::***FileHandle() wrappers to native files.

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

@mgorny

can you at least point me at the setup scripts buildbot us using for netbsd?   
What dependencies are installed?  How is cmake invoked?   I'm getting a ton on 
unrelated problems just trying to build it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68856



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

[Lldb-commits] [lldb] 04edd18 - remove multi-argument form of PythonObject::Reset()

[Lawrence D'Anna via lldb-commits]

Author: Lawrence D'Anna
Date: 2019-10-22T02:32:37Z
New Revision: 04edd1893c2d0f35880fd5f81e78dc23979df0b9

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

LOG: remove multi-argument form of PythonObject::Reset()

Summary:
With this patch, only the no-argument form of `Reset()` remains in
PythonDataObjects.   It also deletes PythonExceptionState in favor of
PythonException, because the only call-site of PythonExceptionState was
also using Reset, so I cleaned up both while I was there.

Reviewers: JDevlieghere, clayborg, labath, jingham

Reviewed By: labath

Subscribers: mgorny, lldb-commits

Tags: #lldb

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

llvm-svn: 375475

Added: 


Modified: 
lldb/include/lldb/Interpreter/ScriptInterpreter.h
lldb/scripts/Python/python-extensions.swig
lldb/scripts/Python/python-typemaps.swig
lldb/scripts/Python/python-wrapper.swig
lldb/scripts/lldb.swig
lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Removed: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.h
lldb/unittests/ScriptInterpreter/Python/PythonExceptionStateTests.cpp



diff  --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h 
b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index 3dd75b558508..23fadf02e591 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -65,6 +65,9 @@ class ScriptInterpreter : public PluginInterface {
 
 bool GetSetLLDBGlobals() const { return m_set_lldb_globals; }
 
+// If this is true then any exceptions raised by the script will be
+// cleared with PyErr_Clear().   If false then they will be left for
+// the caller to clean up
 bool GetMaskoutErrors() const { return m_maskout_errors; }
 
 ExecuteScriptOptions &SetEnableIO(bool enable) {

diff  --git a/lldb/scripts/Python/python-extensions.swig 
b/lldb/scripts/Python/python-extensions.swig
index 7823dc4ad1a0..c10c32b44877 100644
--- a/lldb/scripts/Python/python-extensions.swig
+++ b/lldb/scripts/Python/python-extensions.swig
@@ -8,10 +8,7 @@
 size_t desc_len = description.GetSize();
 if (desc_len > 0 && (desc[desc_len-1] == '\n' || 
desc[desc_len-1] == '\r'))
 --desc_len;
-if (desc_len > 0)
-return lldb_private::PythonString(llvm::StringRef(desc, 
desc_len)).release();
-else
-return lldb_private::PythonString("").release();
+return PythonString(llvm::StringRef(desc, desc_len)).release();
 }
 %clearnothreadallow;
 }
@@ -24,10 +21,7 @@
 size_t desc_len = description.GetSize();
 if (desc_len > 0 && (desc[desc_len-1] == '\n' || 
desc[desc_len-1] == '\r'))
 --desc_len;
-if (desc_len > 0)
-return lldb_private::PythonString(llvm::StringRef(desc, 
desc_len)).release();
-else
-return lldb_private::PythonString("").release();
+return PythonString(llvm::StringRef(desc, desc_len)).release();
 }
 %clearnothreadallow;
 }
@@ -40,10 +34,7 @@
 size_t desc_len = description.GetSize();
 if (desc_len > 0 && (desc[desc_len-1] == '\n' || 
desc[desc_len-1] == '\r'))
 --desc_len;
-if (desc_len > 0)
-return lldb_private::PythonString(llvm::StringRef(desc, 
desc_len)).release();
-else
-return lldb_private::PythonString("").release();
+return PythonString(llvm::StringRef(desc, desc_len)).release();
 }
 %clearnothreadallow;
 
@@ -71,10 +62,7 @@
 size_t desc_len = description.GetSize();
 if (desc_len > 0 && (desc[desc_len-1] == '\n' || 
desc[desc_len-1] == '\r'))
 --desc_len;
-if (desc_len > 0)
-return lldb_private::PythonString(llvm::StringRef(desc, 
desc_len)).release();
-else
-return lldb_private::PythonString("").release();
+return PythonString(llvm::StringRef(desc, desc_len)).release();

[Lldb-commits] [PATCH] D69214: remove multi-argument form of PythonObject::Reset()

[Lawrence D'Anna via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG04edd1893c2d: remove multi-argument form of 
PythonObject::Reset() (authored by lawrence_danna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69214

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/scripts/Python/python-extensions.swig
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/Python/python-wrapper.swig
  lldb/scripts/lldb.swig
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonExceptionState.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonExceptionStateTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonExceptionStateTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonExceptionStateTests.cpp
+++ /dev/null
@@ -1,164 +0,0 @@
-//===-- PythonExceptionStateTest.cpp --*- C++
-//-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "gtest/gtest.h"
-
-#include "Plugins/ScriptInterpreter/Python/PythonDataObjects.h"
-#include "Plugins/ScriptInterpreter/Python/PythonExceptionState.h"
-#include "Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h"
-#include "Plugins/ScriptInterpreter/Python/lldb-python.h"
-
-#include "PythonTestSuite.h"
-
-using namespace lldb_private;
-
-class PythonExceptionStateTest : public PythonTestSuite {
-public:
-protected:
-  void RaiseException() {
-PyErr_SetString(PyExc_RuntimeError, "PythonExceptionStateTest test error");
-  }
-};
-
-TEST_F(PythonExceptionStateTest, TestExceptionStateChecking) {
-  PyErr_Clear();
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  RaiseException();
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestAcquisitionSemantics) {
-  PyErr_Clear();
-  PythonExceptionState no_error(false);
-  EXPECT_FALSE(no_error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-  RaiseException();
-  PythonExceptionState error(false);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  error.Discard();
-
-  PyErr_Clear();
-  RaiseException();
-  error.Acquire(false);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestDiscardSemantics) {
-  PyErr_Clear();
-
-  // Test that discarding an exception does not restore the exception
-  // state even when auto-restore==true is set
-  RaiseException();
-  PythonExceptionState error(true);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  error.Discard();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-}
-
-TEST_F(PythonExceptionStateTest, TestResetSemantics) {
-  PyErr_Clear();
-
-  // Resetting when auto-restore is true should restore.
-  RaiseException();
-  PythonExceptionState error(true);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  error.Reset();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-
-  // Resetting when auto-restore is false should discard.
-  RaiseException();
-  PythonExceptionState error2(false);
-  EXPECT_TRUE(error2.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-  error2.Reset();
-  EXPECT_FALSE(error2.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestManualRestoreSemantics) {
-  PyErr_Clear();
-  RaiseException();
-  PythonExceptionState error(false);
-  EXPECT_TRUE(error.IsError());
-  EXPECT_FALSE(PythonExceptionState::HasErrorOccurred());
-
-  error.Restore();
-  EXPECT_FALSE(error.IsError());
-  EXPECT_TRUE(PythonExceptionState::HasErrorOccurred());
-
-  PyErr_Clear();
-}
-
-TEST_F(PythonExceptionStateTest, TestAutoRestoreSemantics) {
-  PyErr_Clear();
-  // Test that using the auto-restore flag correct

[Lldb-commits] [lldb] d602e0d - fix PythonDataObjectsTest.TestExceptions on windows

[Lawrence D'Anna via lldb-commits]

Author: Lawrence D'Anna
Date: 2019-10-22T04:00:37Z
New Revision: d602e0d0cab270761553c79d2e42b8ac6b756157

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

LOG: fix PythonDataObjectsTest.TestExceptions on windows

Looks like on windows googlemock regexes treat newlines differently
from on darwin.This patch fixes the regex in this test so it
will work on both.

Fixes: https://reviews.llvm.org/D69214
llvm-svn: 375477

Added: 


Modified: 
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Removed: 




diff  --git 
a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp 
b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
index b676b42da666..7481482fd8fc 100644
--- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -805,13 +805,13 @@ main = foo
 
   PythonScript foo(script);
 
-  EXPECT_THAT_EXPECTED(foo(),
-   llvm::Failed(testing::Property(
-   &PythonException::ReadBacktrace,
-   testing::ContainsRegex("line 3, in foo..*"
-  "line 5, in bar.*"
-  "line 7, in baz.*"
-  "ZeroDivisionError";
+  EXPECT_THAT_EXPECTED(
+  foo(), llvm::Failed(testing::Property(
+ &PythonException::ReadBacktrace,
+ testing::AllOf(testing::ContainsRegex("line 3, in foo"),
+testing::ContainsRegex("line 5, in bar"),
+testing::ContainsRegex("line 7, in baz"),
+
testing::ContainsRegex("ZeroDivisionError");
 
   static const char script2[] = R"(
 class MyError(Exception):



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

[Lldb-commits] [PATCH] D68856: convert SBDebugger::***FileHandle() wrappers to native files.

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D68856#1717332 , @lawrence_danna 
wrote:

> I spoke too soon, I don't have a *usable* netbsd VM.
>
> @mgorny, Do you have a machine you can debug on yourself?


Yes.

> Do those errors happen if you run the same test on an interactive terminal?

Yes, if by that you mean lit run on console. I haven't tried running it outside 
lit (yet).

> Can you step through the SWIG wrappers and tell me where it errors out?

I'm planning to do this later today.

In D68856#1717567 , @lawrence_danna 
wrote:

> can you at least point me at the setup scripts buildbot us using for netbsd?  
>  What dependencies are installed?  How is cmake invoked?   I'm getting a ton 
> on unrelated problems just trying to build it.


https://github.com/mgorny/netbsd-llvm-build/tree/master/buildbotScripts/bashShell/svntotbuild

We're using setEnv, checkoutSource, cmake, buildLocal, in this order. I suppose 
only cmake matters to you, the stuff in build is a hack to reduce memory usage. 
Also relevant is that we're running 8.x kernel there (`netbsd-8` branch if 
you're using git).

We install stuff via pkgsrc . Once you do the 
bootstrap,  `/usr/pkgsrc/devel/cmake`, `/usr/pkgsrc/devel/git` and 
`/usr/pkgsrc/devel/swig3` is what you need, optionally 
`/usr/pkgsrc/devel/ninja-build`. I don't recall needing anything else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68856



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

[Lldb-commits] [PATCH] D69230: RFC: specialized Optional for T that can represent its own invalid state

[serge via Phabricator via lldb-commits]

serge-sans-paille added a comment.

(keeping the ball rolling) lYou want to exploit the presence of a sentinel in 
the domain of values of a given type. Maybe make that explicit through the 
method naming instead of using the `isValid` theme.

Some bibliography on the subject: 
https://groups.google.com/a/isocpp.org/forum/#!topic/std-proposals/46J1onhWJ-s/discussion.

On a personal note, as much as I like the idea (and I really do like it), I'd 
rather have llvm::optional sticking to std::optional interface.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69230



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

[Lldb-commits] [PATCH] D69230: RFC: specialized Optional for T that can represent its own invalid state

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

In D69230#1717675 , @serge-sans-paille 
wrote:

> On a personal note, as much as I like the idea (and I really do like it), I'd 
> rather have llvm::optional sticking to std::optional interface.


(short-ish reply, while busy with the dev meeting)
Yep, that's about where I am - I think llvm::Optional should move towards the 
std::optional API. And I don't think maintaining a separate/additional type for 
this functionality is worth the complexity of having a distinct type compared 
to something more and more people will be readily familiar with (std::optional).

But, like serge, I really do like the idea of exposing one or more "secret" 
States from types to use spare bits, padding-ish bits, etc, for DenseMap, 
PointerIntPair/PointerUnion, etc. (be nice to unify these extra state bits, 
even, if possible, across all these types so you could potentially compose them 
- if you've got 3 spare bits in your underlying representation (say, an aligned 
pointer) perhaps you could have a DenseSet> with elements that are 
only sizeof(T*) for instance)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69230



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