[Lldb-commits] [PATCH] D88939: [lldb] Remove unused code in GetVersion (NFC)

2020-10-07 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a reviewer: JDevlieghere.
teemperor added a comment.

LLDB_REVISION and LLDB_REPOSITORY are coming from VCSVersion.inc where it 
should be defined as a string. However the generation of that file seems to be 
broken since the monorepo migration. I put up a patch here that gets this 
working again: D88950 

There is also the framework script that tries to define LLDB_REVISION but I 
assume that's only used for the headers we ship in the Framework? Not 100% sure 
what's going on with that.

Beside that, I think this code can really use some cleanup. I added some 
comments what else can be removed/updated.




Comment at: lldb/source/lldb.cpp:20
 
 static const char *GetLLDBRevision() {
 #ifdef LLDB_REVISION

This can return a string.



Comment at: lldb/source/lldb.cpp:29
 const char *lldb_private::GetVersion() {
   // On platforms other than Darwin, report a version number in the same style
   // as the clang tool.

You can remove that comment too, it's from a time where there was special 
Darwin version code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88939

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


[Lldb-commits] [PATCH] D88387: Create "skinny corefiles" for Mach-O with process save-core / reading

2020-10-07 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

For what it's worth I'm aware of this change, don't hold back on my account. It 
doesn't overlap that much and I think I'll be reworking mine anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88387

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


[Lldb-commits] [PATCH] D87868: [RFC] When calling the process mmap try to call all found instead of just the first one

2020-10-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:54
+if (sc_list.GetContextAtIndex(i, sc) &&
+(sc.symbol->IsExternal() || sc.symbol->IsWeak())) {
   const uint32_t range_scope =

clayborg wrote:
> labath wrote:
> > aadsm wrote:
> > > labath wrote:
> > > > clayborg wrote:
> > > > > aadsm wrote:
> > > > > > clayborg wrote:
> > > > > > > Why are we checking "IsWeak()" here?
> > > > > > A weak symbol is also an external symbol, but it's weak in the 
> > > > > > sense that another external symbol with the same name will take 
> > > > > > precedence over it (as far as I understood).
> > > > > I think we only need to check for external here. Any weak symbol will 
> > > > > also need to be external, but if it isn't we don't want that symbol.
> > > > Your understanding is correct, at least at the object file level -- I'm 
> > > > not sure whether `IsWeak()` implies `IsExternal()` inside lldb. 
> > > > However, I would actually argue for removal of IsWeak() for a different 
> > > > reason -- any weak definition of mmap is not going to be used by the 
> > > > process since libc already has a strong definition of that symbol.
> > > > 
> > > > If we really end up in a situation where we only have a weak mmap 
> > > > symbol around, then this is probably a sufficiently strange setup that 
> > > > we don't want to be randomly calling that function.
> > > All (with the exception of the one overriden by the leak sanitizer) mmap 
> > > symbols in the symbol table are Weak bound but none are External bound, 
> > > this was the main reason that lead me to investigate the difference 
> > > between the two.
> > > 
> > > Not sure how to check how lldb interprets the Weak overall, but I think 
> > > it kind of does, because that's what I saw when I dumped the symbol table:
> > > ```
> > > (lldb) target modules dump symtab libc.so.6
> > >Debug symbol
> > >|Synthetic symbol
> > >||Externally Visible
> > >|||
> > > Index   UserID DSX TypeFile Address/Value Load Address   
> > > Size   Flags  Name
> > > --- -- --- --- -- -- 
> > > -- -- --
> > > ...
> > > [ 6945]   6947   X Code0x0010b5e0 0x769db5e0 
> > > 0x00f9 0x0012 __mmap
> > > ...
> > > ```
> > > 
> > > Another solution I thought was to add a IsLocal to the Symbol (and use 
> > > !IsLocal) but then not really sure how to implement this for all Symbols 
> > > not ELFSymbol.
> > Interesting. I guess I should have verified my assumptions. With that in 
> > mind, I think checking for both weak and external (strong) symbols is fine.
> I think this is a bug in the ObjectFileELF where it is setting weak but not 
> external for weak symbols. Weak symbols would be useless if they weren't 
> external as the first one to get used should end up winning and all other 
> weak symbols should resolve to the first symbol to come along that matches 
> when dyld does a lookup. So I think we should verify this with some linker 
> folks and remove IsWeak from this if statement, and fix the ObjectFileELF 
> symbol table parsing code.
That sounds reasonable. The documentation for SBSymbol::IsExternal says "A read 
only property that returns a boolean value that indicates if this symbol is 
externally visible (exported) from the module that contains it." An elf weak 
symbol definitely fits that definition.

Note that this would be a different meaning of "external" than what is used for 
the "external" llvm [[ https://llvm.org/docs/LangRef.html#linkage-types | 
linkage type ]]. There, it refers to a specific linkage type (externally 
visible and strong), distinct from weak (and linkonce, etc) linkage types, 
though most of them are still externally visible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87868

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


[Lldb-commits] [PATCH] D88906: [lldb/docs] Clarify python/swig version incompatibility

2020-10-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D88906#2314777 , @amccarth wrote:

> Yes, I also (independently) discovered this problem.

Yeah, that seems to be a fairly common problem. I'm gonna try to add some kind 
of a cmake warning about this.




Comment at: lldb/docs/resources/build.rst:81
+` between Python version 3.7 and 
later
+and swig versions older than 4.0.0 which makes builds of lldb using debug
+versions of python unusable. This primarily affects windows, as debug builds of

JDevlieghere wrote:
> The rest of this page seems to be using `LLDB` rather than `lldb`. I have no 
> preference as long as things are consistent.
sounds good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88906

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


[Lldb-commits] [PATCH] D88728: [lldb] Check for and use ptsname_r if available

2020-10-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D88728#2315648 , @MaskRay wrote:

> LGTM. Worth mentioning that this will be in POSIX issue 8 
> https://www.austingroupbugs.net/bug_view_page.php?bug_id=508

Oh, cool. I didn't know posix is still being worked on :P




Comment at: lldb/source/Host/common/PseudoTerminal.cpp:149
+  int r = ptsname_r(m_primary_fd, buf, sizeof(buf));
+  assert(r == 0);
+  return buf;

MaskRay wrote:
> labath wrote:
> > MaskRay wrote:
> > > MaskRay wrote:
> > > > labath wrote:
> > > > > mgorny wrote:
> > > > > > labath wrote:
> > > > > > > mgorny wrote:
> > > > > > > > I would really feel better with a real error handling here. It 
> > > > > > > > shouldn't be hard to use `ErrorOr` here.
> > > > > > > Yeah, but what are you going to do with that value? Pass it to 
> > > > > > > the caller? The existing callers are ignoring the error return 
> > > > > > > anyway, and I don't want to add error handling everywhere as 
> > > > > > > afaict, this function can't fail unless the user messes up the 
> > > > > > > master state (which is not something I want to support).
> > > > > > I get your point but I've literally wasted days because of missing 
> > > > > > error handling, so I'd really preferred if we wouldn't make it even 
> > > > > > worse. Though I guess `assert` is good enough.
> > > > > In some ways it's even better because it will point you straight to 
> > > > > the place where the assumption is violated, whereas a propagated 
> > > > > logic error can manifest itself much farther away (or not at all). :)
> > > > If `ptsname/ptsname_r` fails, buf will be uninitialized and trigger a 
> > > > use-of-uninitialized-value error.
> > > ... in a -DLLVM_ENABLE_ASSERTIONS=off build.
> > > 
> > > This probably still needs some protection.
> > What kind of protection did you have it mind? Initialize the buffer to an 
> > empty string?
> In the case of a non-zero return value, `buf[0] = '\0'` is probably 
> sufficient to avoid an uninitialized value.
sounds good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88728

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


[Lldb-commits] [lldb] 029290f - [lldb/docs] Clarify python/swig version incompatibility

2020-10-07 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-10-07T15:29:29+02:00
New Revision: 029290f1a6231507b82981d56c0a0d2b02d508e0

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

LOG: [lldb/docs] Clarify python/swig version incompatibility

The problematic combo is a debug python>=3.7 && swig<4.0.

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

Added: 


Modified: 
lldb/docs/resources/build.rst

Removed: 




diff  --git a/lldb/docs/resources/build.rst b/lldb/docs/resources/build.rst
index e22db7f6d8f9..b4e58ca977a9 100644
--- a/lldb/docs/resources/build.rst
+++ b/lldb/docs/resources/build.rst
@@ -76,6 +76,12 @@ commands below.
   > pkgin install swig python27 cmake ninja-build
   > brew install swig cmake ninja
 
+Note that there's an `incompatibility
+` between Python version 3.7 and 
later
+and swig versions older than 4.0.0 which makes builds of LLDB using debug
+versions of python unusable. This primarily affects Windows, as debug builds of
+LLDB must use debug python as well.
+
 Windows
 ***
 
@@ -83,10 +89,9 @@ Windows
 * The latest Windows SDK.
 * The Active Template Library (ATL).
 * `GnuWin32 `_ for CoreUtils and Make.
-* `Python 3.6 or 3.8 `_. Python 3.7
-  is known to be incompatible. Make sure to (1) get the x64 variant if that's
-  what you're targetting and (2) install the debug library if you want to build
-  a debug lldb.
+* `Python 3 `_.  Make sure to (1) 
get
+  the x64 variant if that's what you're targetting and (2) install the debug
+  library if you want to build a debug lldb.
 * `Python Tools for Visual Studio
   `_. If you plan to debug test
   failures or even write new tests at all, PTVS is an indispensable debugging



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


[Lldb-commits] [lldb] 3dfb949 - [lldb] Check for and use ptsname_r if available

2020-10-07 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-10-07T15:29:29+02:00
New Revision: 3dfb94986170c57d9b3f5f2cba039a2eab5e6f13

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

LOG: [lldb] Check for and use ptsname_r if available

ptsname is not thread-safe. ptsname_r is available on most (but not all)
systems -- use it preferentially.

In the patch I also improve the thread-safety of the ptsname fallback
path by wrapping it in a mutex. This should guarantee the safety of a
typical ptsname implementation using a single static buffer, as long as
all callers go through this function.

I also remove the error arguments, as the only way this function can
fail is if the "primary" fd is not valid. This is a programmer error as
this requirement is documented, and all callers ensure that is the case.

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

Added: 


Modified: 
lldb/cmake/modules/LLDBGenerateConfig.cmake
lldb/include/lldb/Host/Config.h.cmake
lldb/include/lldb/Host/PseudoTerminal.h
lldb/source/Host/common/ProcessLaunchInfo.cpp
lldb/source/Host/common/PseudoTerminal.cpp

lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Removed: 




diff  --git a/lldb/cmake/modules/LLDBGenerateConfig.cmake 
b/lldb/cmake/modules/LLDBGenerateConfig.cmake
index 0d3a7fdb1816..caeb3969002b 100644
--- a/lldb/cmake/modules/LLDBGenerateConfig.cmake
+++ b/lldb/cmake/modules/LLDBGenerateConfig.cmake
@@ -7,6 +7,7 @@ include(CheckLibraryExists)
 
 set(CMAKE_REQUIRED_DEFINITIONS -D_GNU_SOURCE)
 check_symbol_exists(ppoll poll.h HAVE_PPOLL)
+check_symbol_exists(ptsname_r stdlib.h HAVE_PTSNAME_R)
 set(CMAKE_REQUIRED_DEFINITIONS)
 check_symbol_exists(sigaction signal.h HAVE_SIGACTION)
 check_cxx_symbol_exists(accept4 "sys/socket.h" HAVE_ACCEPT4)

diff  --git a/lldb/include/lldb/Host/Config.h.cmake 
b/lldb/include/lldb/Host/Config.h.cmake
index 7467f429b628..671d71d1c4e3 100644
--- a/lldb/include/lldb/Host/Config.h.cmake
+++ b/lldb/include/lldb/Host/Config.h.cmake
@@ -20,6 +20,8 @@
 
 #cmakedefine01 HAVE_PPOLL
 
+#cmakedefine01 HAVE_PTSNAME_R
+
 #cmakedefine01 HAVE_SIGACTION
 
 #cmakedefine01 HAVE_PROCESS_VM_READV

diff  --git a/lldb/include/lldb/Host/PseudoTerminal.h 
b/lldb/include/lldb/Host/PseudoTerminal.h
index 8a5a233e7748..c2258f15e551 100644
--- a/lldb/include/lldb/Host/PseudoTerminal.h
+++ b/lldb/include/lldb/Host/PseudoTerminal.h
@@ -105,20 +105,11 @@ class PseudoTerminal {
   /// A primary pseudo terminal should already be valid prior to
   /// calling this function.
   ///
-  /// \param[out] error_str
-  /// An pointer to an error that can describe any errors that
-  /// occur. This can be NULL if no error status is desired.
-  ///
   /// \return
-  /// The name of the secondary pseudo terminal as a NULL terminated
-  /// C. This string that comes from static memory, so a copy of
-  /// the string should be made as subsequent calls can change
-  /// this value. NULL is returned if this object doesn't have
-  /// a valid primary pseudo terminal opened or if the call to
-  /// \c ptsname() fails.
+  /// The name of the secondary pseudo terminal.
   ///
   /// \see PseudoTerminal::OpenFirstAvailablePrimary()
-  const char *GetSecondaryName(char *error_str, size_t error_len) const;
+  std::string GetSecondaryName() const;
 
   /// Open the first available pseudo terminal.
   ///

diff  --git a/lldb/source/Host/common/ProcessLaunchInfo.cpp 
b/lldb/source/Host/common/ProcessLaunchInfo.cpp
index 4bc8cda7a006..a4729a28ce74 100644
--- a/lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ b/lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -222,7 +222,7 @@ llvm::Error ProcessLaunchInfo::SetUpPtyRedirection() {
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"PTY::OpenFirstAvailablePrimary failed");
   }
-  const FileSpec secondary_file_spec(m_pty->GetSecondaryName(nullptr, 0));
+  const FileSpec secondary_file_spec(m_pty->GetSecondaryName());
 
   // Only use the secondary tty if we don't have anything specified for
   // input and don't have an action for stdin

diff  --git a/lldb/source/Host/common/PseudoTerminal.cpp 
b/lldb/source/Host/common/PseudoTerminal.cpp
index 72549f1c88ab..4668b09f4fdb 100644
--- a/lldb/source/Host/common/PseudoTerminal.cpp
+++ b/lldb/source/Host/common/PseudoTerminal.cpp
@@ -8,9 +8,11 @@
 
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/Config.h"
-
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/Errno.h"
-
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -128,15 +130,8 @@ bool PseudoTermin

[Lldb-commits] [PATCH] D88728: [lldb] Check for and use ptsname_r if available

2020-10-07 Thread Pavel Labath via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3dfb94986170: [lldb] Check for and use ptsname_r if 
available (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D88728?vs=295793&id=296658#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88728

Files:
  lldb/cmake/modules/LLDBGenerateConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/PseudoTerminal.h
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Host/common/PseudoTerminal.cpp
  
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
  lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -818,7 +818,7 @@
 // does a lot of output.
 if ((!stdin_file_spec || !stdout_file_spec || !stderr_file_spec) &&
 pty.OpenFirstAvailablePrimary(O_RDWR | O_NOCTTY, nullptr, 0)) {
-  FileSpec secondary_name{pty.GetSecondaryName(nullptr, 0)};
+  FileSpec secondary_name(pty.GetSecondaryName());
 
   if (!stdin_file_spec)
 stdin_file_spec = secondary_name;
Index: lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
@@ -380,8 +380,7 @@
   FileSpec stdout_file_spec{};
   FileSpec stderr_file_spec{};
 
-  const FileSpec dbg_pts_file_spec{
-  launch_info.GetPTY().GetSecondaryName(NULL, 0)};
+  const FileSpec dbg_pts_file_spec{launch_info.GetPTY().GetSecondaryName()};
 
   file_action = launch_info.GetFileActionForFD(STDIN_FILENO);
   stdin_file_spec =
Index: lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===
--- lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -407,25 +407,21 @@
 const int master_fd = launch_info.GetPTY().GetPrimaryFileDescriptor();
 if (master_fd != PseudoTerminal::invalid_fd) {
   // Check in case our file action open wants to open the secondary
-  const char *secondary_path =
-  launch_info.GetPTY().GetSecondaryName(NULL, 0);
-  if (secondary_path) {
-FileSpec secondary_spec(secondary_path);
-if (file_spec == secondary_spec) {
-  int secondary_fd =
-  launch_info.GetPTY().GetSecondaryFileDescriptor();
-  if (secondary_fd == PseudoTerminal::invalid_fd)
-secondary_fd =
-launch_info.GetPTY().OpenSecondary(O_RDWR, nullptr, 0);
-  if (secondary_fd == PseudoTerminal::invalid_fd) {
-error.SetErrorStringWithFormat(
-"unable to open secondary pty '%s'", secondary_path);
-return error; // Failure
-  }
-  [options setValue:[NSNumber numberWithInteger:secondary_fd]
- forKey:key];
-  return error; // Success
+  FileSpec secondary_spec(launch_info.GetPTY().GetSecondaryName());
+  if (file_spec == secondary_spec) {
+int secondary_fd =
+launch_info.GetPTY().GetSecondaryFileDescriptor();
+if (secondary_fd == PseudoTerminal::invalid_fd)
+  secondary_fd =
+  launch_info.GetPTY().OpenSecondary(O_RDWR, nullptr, 0);
+if (secondary_fd == PseudoTerminal::invalid_fd) {
+  error.SetErrorStringWithFormat(
+  "unable to open secondary pty '%s'", secondary_path);
+  return error; // Failure
 }
+[options setValue:[NSNumber numberWithInteger:secondary_fd]
+   forKey:key];
+return error; // Success
   }
 }
 Status posix_error;
Index: lldb/source/Host/common/PseudoTerminal.cpp
===
--- lldb/source/Host/common/PseudoTerminal.cpp
+++ lldb/source/Host/common/PseudoTerminal.cpp
@@ -8,9 +8,11 @@
 
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/Config.h"
-
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/Errno.h"
-
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -128,15 +130,8 @@
 
   CloseSecondaryFileDescriptor();
 
-  // Open 

[Lldb-commits] [PATCH] D88906: [lldb/docs] Clarify python/swig version incompatibility

2020-10-07 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG029290f1a623: [lldb/docs] Clarify python/swig version 
incompatibility (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D88906?vs=296474&id=296657#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88906

Files:
  lldb/docs/resources/build.rst


Index: lldb/docs/resources/build.rst
===
--- lldb/docs/resources/build.rst
+++ lldb/docs/resources/build.rst
@@ -76,6 +76,12 @@
   > pkgin install swig python27 cmake ninja-build
   > brew install swig cmake ninja
 
+Note that there's an `incompatibility
+` between Python version 3.7 and 
later
+and swig versions older than 4.0.0 which makes builds of LLDB using debug
+versions of python unusable. This primarily affects Windows, as debug builds of
+LLDB must use debug python as well.
+
 Windows
 ***
 
@@ -83,10 +89,9 @@
 * The latest Windows SDK.
 * The Active Template Library (ATL).
 * `GnuWin32 `_ for CoreUtils and Make.
-* `Python 3.6 or 3.8 `_. Python 3.7
-  is known to be incompatible. Make sure to (1) get the x64 variant if that's
-  what you're targetting and (2) install the debug library if you want to build
-  a debug lldb.
+* `Python 3 `_.  Make sure to (1) 
get
+  the x64 variant if that's what you're targetting and (2) install the debug
+  library if you want to build a debug lldb.
 * `Python Tools for Visual Studio
   `_. If you plan to debug test
   failures or even write new tests at all, PTVS is an indispensable debugging


Index: lldb/docs/resources/build.rst
===
--- lldb/docs/resources/build.rst
+++ lldb/docs/resources/build.rst
@@ -76,6 +76,12 @@
   > pkgin install swig python27 cmake ninja-build
   > brew install swig cmake ninja
 
+Note that there's an `incompatibility
+` between Python version 3.7 and later
+and swig versions older than 4.0.0 which makes builds of LLDB using debug
+versions of python unusable. This primarily affects Windows, as debug builds of
+LLDB must use debug python as well.
+
 Windows
 ***
 
@@ -83,10 +89,9 @@
 * The latest Windows SDK.
 * The Active Template Library (ATL).
 * `GnuWin32 `_ for CoreUtils and Make.
-* `Python 3.6 or 3.8 `_. Python 3.7
-  is known to be incompatible. Make sure to (1) get the x64 variant if that's
-  what you're targetting and (2) install the debug library if you want to build
-  a debug lldb.
+* `Python 3 `_.  Make sure to (1) get
+  the x64 variant if that's what you're targetting and (2) install the debug
+  library if you want to build a debug lldb.
 * `Python Tools for Visual Studio
   `_. If you plan to debug test
   failures or even write new tests at all, PTVS is an indispensable debugging
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D88967: [lldb] Add a cmake warning about the python/swig incompatibility

2020-10-07 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: amccarth, tatyana-krasnukha, JDevlieghere.
Herald added a subscriber: mgorny.
Herald added a project: LLDB.
labath requested review of this revision.

Raise awareness of the fact that some versions of swig and python (and
build types) just don't mix.

One day this will be a reason to require swig>=4.0, but this version is
too hot off the press right now..


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88967

Files:
  lldb/cmake/modules/FindPythonAndSwig.cmake


Index: lldb/cmake/modules/FindPythonAndSwig.cmake
===
--- lldb/cmake/modules/FindPythonAndSwig.cmake
+++ lldb/cmake/modules/FindPythonAndSwig.cmake
@@ -45,6 +45,17 @@
 message(STATUS "SWIG 2 or later is required for Python support in LLDB but 
could not be found")
   endif()
 
+  get_property(MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
+  if ("${Python3_VERSION}" VERSION_GREATER_EQUAL "3.7" AND
+  "${SWIG_VERSION}" VERSION_LESS "4.0" AND WIN32 AND (
+  ${MULTI_CONFIG} OR (${uppercase_CMAKE_BUILD_TYPE} STREQUAL "DEBUG")))
+# Technically this can happen with non-Windows builds too, but we are not
+# able to detect whether Python was built with assertions, and only Windows
+# has the requirement that Debug LLDB must use Debug Python.
+message(WARNING "Debug builds of LLDB are likely to be unusable due to "
+  ". Please use SWIG >= 4.0.")
+  endif()
+
   include(FindPackageHandleStandardArgs)
   find_package_handle_standard_args(PythonAndSwig
 FOUND_VAR


Index: lldb/cmake/modules/FindPythonAndSwig.cmake
===
--- lldb/cmake/modules/FindPythonAndSwig.cmake
+++ lldb/cmake/modules/FindPythonAndSwig.cmake
@@ -45,6 +45,17 @@
 message(STATUS "SWIG 2 or later is required for Python support in LLDB but could not be found")
   endif()
 
+  get_property(MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
+  if ("${Python3_VERSION}" VERSION_GREATER_EQUAL "3.7" AND
+  "${SWIG_VERSION}" VERSION_LESS "4.0" AND WIN32 AND (
+  ${MULTI_CONFIG} OR (${uppercase_CMAKE_BUILD_TYPE} STREQUAL "DEBUG")))
+# Technically this can happen with non-Windows builds too, but we are not
+# able to detect whether Python was built with assertions, and only Windows
+# has the requirement that Debug LLDB must use Debug Python.
+message(WARNING "Debug builds of LLDB are likely to be unusable due to "
+  ". Please use SWIG >= 4.0.")
+  endif()
+
   include(FindPackageHandleStandardArgs)
   find_package_handle_standard_args(PythonAndSwig
 FOUND_VAR
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D87442: [lldb] Show flags for memory regions

2020-10-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D87442#2300987 , @labath wrote:

> I think this patch for its test coverage.

And by "think", I meant "like". :)

In D87442#2311798 , @DavidSpickett 
wrote:

>> The thing I'm not so sure about is the verbatim passing of the os-specific 
>> flags. That's nice for the purpose of displaying them to the user, but it 
>> can make things messy for programatic use, particularly if multiple 
>> platforms start using these. What's do you intend to do with these flags? 
>> Like how many of them are you going to actually use, and for what purpose?
>
> At this point I only need to get the "mt" flag that shows whether a region 
> has memory tagging enabled. This will be used by lldb internally and anyone 
> checking for memory tagging with "memory region".
>
> Some background on the design...
>
> With the current implementation:
>
> - We don't have to translate the flags per OS to show them.
> - Has() needs to check for OS specific names, which balances things 
> out. (you've got an OS specific switch either way)
> - The gdb protocol packet just contains the names as strings.
> - OSes that don't present flags as strings directly in their API have to 
> provide string versions.
>
> At first I did have a generically named bitset of flags internally. This 
> means that:
>
> - In the "memory region" output the user can't tell the difference between an 
> unrecognised flag, or a flag that's not enabled. Since we'd need to update 
> lldb to handle the new flag. (a rare occurrence now that I think about it)
> - When "memory region" prints we'd need to translate those flags to OS 
> specific names, or use long generic names like "memory tagging: enabled". (I 
> prefer the former so you could compare it directly to the native tools)
> - A region object's Has() is the same code regardless of OS.
> - The gdb protocol packet would contain generic versions of the flag names.
>
> Now that I wrote that the second idea does look cleaner. New flags are going 
> to be infrequent and internal calls won't have to care about what OS they're 
> on to query flags.
>
> Is that the direction you were thinking of? If so I'll rework this to do that 
> instead.

Yes, that's pretty much it. I sympathise with wanting to match the native 
tools, but that's not something lldb is very good at right now (and there's an 
opposite drive to make lldb behavior consistent across platforms). For now I'd 
just put on that and have lldb choose an "os-independent" flag name which 
"happens" to match the linux name (one of the advantages of being first).

That said, with this and @jasonmolenda's change to memory regions, I think the 
description of a single memory region becomes too complicated to reasonably fit 
on a single line. I think we might want to change the output of `memory region` 
to span multiple lines, at which point, the string "memory tagging: enabled" 
might not look too bad (we could add headings for other properties too).




Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1301-1302
   Status Result;
-  ParseLinuxMapRegions(BufferOrError.get()->getBuffer(),
-   [&](const MemoryRegionInfo &Info, const Status &ST) {
- if (ST.Success()) {
-   FileSpec file_spec(Info.GetName().GetCString());
-   FileSystem::Instance().Resolve(file_spec);
-   m_mem_region_cache.emplace_back(Info, file_spec);
-   return true;
- } else {
-   m_supports_mem_region = LazyBool::eLazyBoolNo;
-   LLDB_LOG(log, "failed to parse proc maps: {0}", ST);
-   Result = ST;
-   return false;
- }
-   });
-  if (Result.Fail())
-return Result;
+  LinuxMapCallback callback = [&](const llvm::Optional &Info,
+  const Status &ST) {
+if (ST.Success()) {

If we're changing the signature, we might as well make this 
`Expected`.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1323
+  return Result;
+}
+  } else {

We normally don't put braces around simple if statements like this: 




Comment at: lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp:160
+// If this line is a property line
+if (name.find(' ') == llvm::StringRef::npos) {
+  if (region) {

`name.contains(' ')`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442

___
lldb-commi

[Lldb-commits] [PATCH] D88939: [lldb] Remove unused code in GetVersion (NFC)

2020-10-07 Thread Dave Lee via Phabricator via lldb-commits
kastiglione updated this revision to Diff 296695.
kastiglione added a comment.

Restore repository handling


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88939

Files:
  lldb/source/lldb.cpp


Index: lldb/source/lldb.cpp
===
--- lldb/source/lldb.cpp
+++ lldb/source/lldb.cpp
@@ -33,12 +33,7 @@
 #endif
 }
 
-#define QUOTE(str) #str
-#define EXPAND_AND_QUOTE(str) QUOTE(str)
-
 const char *lldb_private::GetVersion() {
-  // On platforms other than Darwin, report a version number in the same style
-  // as the clang tool.
   static std::string g_version_str;
   if (g_version_str.empty()) {
 g_version_str += "lldb version ";


Index: lldb/source/lldb.cpp
===
--- lldb/source/lldb.cpp
+++ lldb/source/lldb.cpp
@@ -33,12 +33,7 @@
 #endif
 }
 
-#define QUOTE(str) #str
-#define EXPAND_AND_QUOTE(str) QUOTE(str)
-
 const char *lldb_private::GetVersion() {
-  // On platforms other than Darwin, report a version number in the same style
-  // as the clang tool.
   static std::string g_version_str;
   if (g_version_str.empty()) {
 g_version_str += "lldb version ";
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D88939: [lldb] Remove unused code in GetVersion (NFC)

2020-10-07 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added a comment.

@teemperor I removed the comment and restored repository handling. It becomes 
much more of a useless diff :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88939

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


[Lldb-commits] [PATCH] D88939: [lldb] Remove unused code in GetVersion (NFC)

2020-10-07 Thread Dave Lee via Phabricator via lldb-commits
kastiglione updated this revision to Diff 296700.
kastiglione added a comment.

Update commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88939

Files:
  lldb/source/lldb.cpp


Index: lldb/source/lldb.cpp
===
--- lldb/source/lldb.cpp
+++ lldb/source/lldb.cpp
@@ -33,12 +33,7 @@
 #endif
 }
 
-#define QUOTE(str) #str
-#define EXPAND_AND_QUOTE(str) QUOTE(str)
-
 const char *lldb_private::GetVersion() {
-  // On platforms other than Darwin, report a version number in the same style
-  // as the clang tool.
   static std::string g_version_str;
   if (g_version_str.empty()) {
 g_version_str += "lldb version ";


Index: lldb/source/lldb.cpp
===
--- lldb/source/lldb.cpp
+++ lldb/source/lldb.cpp
@@ -33,12 +33,7 @@
 #endif
 }
 
-#define QUOTE(str) #str
-#define EXPAND_AND_QUOTE(str) QUOTE(str)
-
 const char *lldb_private::GetVersion() {
-  // On platforms other than Darwin, report a version number in the same style
-  // as the clang tool.
   static std::string g_version_str;
   if (g_version_str.empty()) {
 g_version_str += "lldb version ";
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D88975: [LLDB] On Windows, fix tests

2020-10-07 Thread Alexandre Ganea via Phabricator via lldb-commits
aganea created this revision.
aganea added reviewers: amccarth, labath.
Herald added a reviewer: JDevlieghere.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
aganea requested review of this revision.

This patch fixes a few issues seen when running `ninja check-lldb` in a Release 
build:

- Some binaries couldn't be found (such as lldb-vscode.exe), because `.exe` 
wasn't appended to the file name.
- Many tests used to fail since the OS error messages are not emitted in 
English, because of our installed French locale.
- Also, because of our codepage being Windows-1252, python failed to decode the 
messages.

After this patch, all tests pass with VS2017.
When building with VS2019 or Clang 10, the following tests still //do not 
pass//:

  lldb-api :: python_api/lldbutil/iter/TestLLDBIterator.py
  lldb-shell :: SymbolFile/PDB/enums-layout.test
  lldb-shell :: SymbolFile/PDB/typedefs.test
  lldb-unit :: 
tools/lldb-server/tests/./LLDBServerTests.exe/StandardStartupTest.TestStopReplyContainsThreadPcs

For the first three tests above, this has to do with the debug info not being 
emitted as expected. I'm not sure about the last one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88975

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test_event/build_exception.py
  lldb/test/API/commands/target/basic/TestTargetCommand.py
  lldb/unittests/Utility/StatusTest.cpp


Index: lldb/unittests/Utility/StatusTest.cpp
===
--- lldb/unittests/Utility/StatusTest.cpp
+++ lldb/unittests/Utility/StatusTest.cpp
@@ -10,7 +10,7 @@
 #include "gtest/gtest.h"
 
 #ifdef _WIN32
-#include 
+#include 
 #endif
 
 using namespace lldb_private;
@@ -71,6 +71,14 @@
   EXPECT_FALSE(success.ToError());
   EXPECT_TRUE(success.Success());
 
+  WCHAR name[128]{};
+  GetLocaleInfoEx(LOCALE_NAME_USER_DEFAULT, LOCALE_SNAME, (LPWSTR)&name,
+  sizeof(name) / sizeof(WCHAR));
+  // Skip the following tests on non-English, non-US, locales because the
+  // formatted messages will be different.
+  if (wcscmp(L"en-US", name) != 0)
+return;
+
   auto s = Status(ERROR_ACCESS_DENIED, ErrorType::eErrorTypeWin32);
   EXPECT_TRUE(s.Fail());
   EXPECT_STREQ("Access is denied. ", s.AsCString());
Index: lldb/test/API/commands/target/basic/TestTargetCommand.py
===
--- lldb/test/API/commands/target/basic/TestTargetCommand.py
+++ lldb/test/API/commands/target/basic/TestTargetCommand.py
@@ -353,7 +353,7 @@
 @no_debug_info_test
 def test_target_create_nonexistent_core_file(self):
 self.expect("target create -c doesntexist", error=True,
-patterns=["Cannot open 'doesntexist'", ": (No such file or 
directory|The system cannot find the file specified)"])
+patterns=["Cannot open 'doesntexist'", ": (No such file or 
directory|The system cannot find the file specified|Le fichier 
sp\udce9cifi\udce9 est introuvable)"])
 
 # Write only files don't seem to be supported on Windows.
 @skipIfWindows
@@ -368,7 +368,7 @@
 @no_debug_info_test
 def test_target_create_nonexistent_sym_file(self):
 self.expect("target create -s doesntexist doesntexisteither", 
error=True,
-patterns=["Cannot open '", ": (No such file or 
directory|The system cannot find the file specified)"])
+patterns=["Cannot open '", ": (No such file or 
directory|The system cannot find the file specified|Le fichier 
sp\udce9cifi\udce9 est introuvable)"])
 
 @skipIfWindows
 @no_debug_info_test
Index: lldb/packages/Python/lldbsuite/test_event/build_exception.py
===
--- lldb/packages/Python/lldbsuite/test_event/build_exception.py
+++ lldb/packages/Python/lldbsuite/test_event/build_exception.py
@@ -12,4 +12,4 @@
 @staticmethod
 def format_build_error(command, command_output):
 return "Error when building test subject.\n\nBuild 
Command:\n{}\n\nBuild Command Output:\n{}".format(
-command, command_output.decode("utf-8"))
+command, command_output.decode("utf-8", errors='ignore'))
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -52,6 +52,9 @@
 """Returns true if fpath is an executable."""
 if fpath == None:
 return False
+if sys.platform == 'win32':
+if not fpath.endswith(".exe"):
+fpath += ".exe"
 return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
 
 


Index: lldb/unittests/Utility/StatusTest.cpp
===
--- lldb/unittests/Utility/StatusTest.cpp
+++ lldb/unittests/Utility/StatusTest.cpp
@@ -

[Lldb-commits] [PATCH] D87442: [lldb] Show flags for memory regions

2020-10-07 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett planned changes to this revision.
DavidSpickett added a comment.

> Yes, that's pretty much it.

Great, I will rework this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442

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


[Lldb-commits] [PATCH] D88975: [LLDB] On Windows, fix tests

2020-10-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Is there a way to force a program to use a specific locale, ignoring the 
system-wide setting (something like `export LC_ALL=C` on unix)? Hardcoding the 
error message for all locales is not a viable long term strategy... :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88975

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


[Lldb-commits] [PATCH] D88992: [lldb] Fix "frame var" for large bitfields

2020-10-07 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: jingham, teemperor.
Herald added a reviewer: JDevlieghere.
Herald added a project: LLDB.
labath requested review of this revision.

The problem here is in the "sliding" code in
ValueObjectChild::UpdateValue. It modifies m_bitfield_bit_offset and
m_value to ensure the bitfield value fits the window given by the
underlying type.

However, this is broken next time UpdateValue is called, because it
updates the m_value value from the parent. However, the value cannot be
slid again because the m_bitfield_bit_offset is already modified.

It seems this can happen only under specific circumstances. One way to
trigger is is to run an expression which can be interpreted (jitting it
causes a new StackFrame and ValueObject variables to be created).

I fix this bug by modifying m_byte_offset instead of m_scalar, and
ensuring the changes are folded into m_scalar regardless of how many
times UpdateValue is called.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88992

Files:
  lldb/source/Core/ValueObjectChild.cpp
  lldb/test/API/lang/c/bitfields/TestBitfields.py


Index: lldb/test/API/lang/c/bitfields/TestBitfields.py
===
--- lldb/test/API/lang/c/bitfields/TestBitfields.py
+++ lldb/test/API/lang/c/bitfields/TestBitfields.py
@@ -147,6 +147,27 @@
 self.expect("v/x large_packed", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["a = 0x000c", "b = 
0x000deee"])
 
+# BitFields exhibit crashes in record layout on Windows
+# (http://llvm.org/pr21800)
+@skipIfWindows
+def test_pr47743(self):
+# Ensure evaluating (emulating) an expression does not break bitfield
+# values for already parsed variables. The expression is run twice
+# because the very first expression can resume a target (to allocate
+# memory, etc.) even if it is not being jitted.
+self.build()
+lldbutil.run_to_line_breakpoint(self, lldb.SBFileSpec("main.c"),
+self.line)
+self.expect("v/x large_packed", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=["a = 0x000c", "b = 
0x000deee"])
+self.expect("expr --allow-jit false  -- more_bits.a", 
VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['uint32_t', '3'])
+self.expect("v/x large_packed", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=["a = 0x000c", "b = 
0x000deee"])
+self.expect("expr --allow-jit false  -- more_bits.a", 
VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['uint32_t', '3'])
+self.expect("v/x large_packed", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=["a = 0x000c", "b = 
0x000deee"])
 
 @add_test_categories(['pyapi'])
 # BitFields exhibit crashes in record layout on Windows
Index: lldb/source/Core/ValueObjectChild.cpp
===
--- lldb/source/Core/ValueObjectChild.cpp
+++ lldb/source/Core/ValueObjectChild.cpp
@@ -165,10 +165,6 @@
   } else if (addr == 0) {
 m_error.SetErrorString("parent is NULL");
   } else {
-// Set this object's scalar value to the address of its value by
-// adding its byte offset to the parent address
-m_value.GetScalar() += GetByteOffset();
-
 // If a bitfield doesn't fit into the child_byte_size'd
 // window at child_byte_offset, move the window forward
 // until it fits.  The problem here is that Value has no
@@ -187,11 +183,15 @@
 if (bitfield_end > *type_bit_size) {
   uint64_t overhang_bytes =
   (bitfield_end - *type_bit_size + 7) / 8;
-  m_value.GetScalar() += overhang_bytes;
+  m_byte_offset += overhang_bytes;
   m_bitfield_bit_offset -= overhang_bytes * 8;
 }
   }
 }
+
+// Set this object's scalar value to the address of its value by
+// adding its byte offset to the parent address
+m_value.GetScalar() += GetByteOffset();
   }
 } break;
 


Index: lldb/test/API/lang/c/bitfields/TestBitfields.py
===
--- lldb/test/API/lang/c/bitfields/TestBitfields.py
+++ lldb/test/API/lang/c/bitfields/TestBitfields.py
@@ -147,6 +147,27 @@
 self.expect("v/x large_packed", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=["a = 0x000c", "b = 0x000deee"])
 
+# BitFields exhibit crashes in record layout on Windows
+# (http://llvm.org/pr21800)
+@skipIfWindows
+def test_pr47743(self):
+# Ensure evaluating (emulating) an expression does not break bitfield
+# values for already parsed variables. The 

[Lldb-commits] [PATCH] D88975: [LLDB] On Windows, fix tests

2020-10-07 Thread Adrian McCarthy via Phabricator via lldb-commits
amccarth added a comment.

It's interesting that I haven't encountered some of these errors.  There are 
five _other_ lldb tests that do fail for me.  I have a fix in the works for 
some of those.

I agree with @labath that including error message patterns in various languages 
isn't scalable. Since dotest,py is starting up the processes under test, 
perhaps there's a way to force it to use a particular locale.  Besides the 
language of system error messages, locales can change the format of numbers, 
dates, etc.

Unfortunately, I don't think it's as simple as an environment variable.  I 
expect this is driven by the user's (or system default) locales settings as 
tracked by Windows.  That's distinct from the C runtime concept of locales.

Maybe dotest.py could change its own Windows locale setting and the processes 
it spawns would inherit that.  I don't know if that would work, but I don't see 
a good alternative.




Comment at: lldb/unittests/Utility/StatusTest.cpp:80
+  if (wcscmp(L"en-US", name) != 0)
+return;
+

Rather than an early return, perhaps the code should still be exercised, but 
the language-specific EXPECTs could be skipped.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88975

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


[Lldb-commits] [PATCH] D88967: [lldb] Add a cmake warning about the python/swig incompatibility

2020-10-07 Thread Adrian McCarthy via Phabricator via lldb-commits
amccarth added a comment.

If I recall correctly, the non-debug builds still had the problem, they just 
didn't have the assertion that made it obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88967

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


[Lldb-commits] [PATCH] D88967: [lldb] Add a cmake warning about the python/swig incompatibility

2020-10-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D88967#2317522 , @amccarth wrote:

> If I recall correctly, the non-debug builds still had the problem, they just 
> didn't have the assertion that made it obvious.

Is that problem only theoretical (like, "you shouldn't be doing that") or does 
it have some practical consequences (crashes, incorrect operation, etc.)?

I'm certain that it is a "theoretical" problem, but AFAICT it doesn't have any 
practical consequences. I've been running python-3.8+swig-3.0.12 for quite some 
time now any I didn't observe see any issues. And I know a lot of other people 
are running a similar combo too..

I think the "benignness" of this assertion failure was the reason it took so 
long to find/fix this bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88967

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


[Lldb-commits] [PATCH] D88975: [LLDB] On Windows, fix tests

2020-10-07 Thread Alexandre Ganea via Phabricator via lldb-commits
aganea added a comment.

In D88975#2317494 , @amccarth wrote:

> Maybe dotest.py could change its own Windows locale setting and the processes 
> it spawns would inherit that.  I don't know if that would work, but I don't 
> see a good alternative.

One problem is that some users might //not// have the English language pack 
installed. I only have `fr-CA` and `en-CA` installed but not `en-US`. There's 
absolutely no guarantee that there will be a `en-XX` package available. I'll do 
some more testing and upload a new diff, but I took a defensive approach, like 
for `StatusTest.cpp`, where the two tests below in `TestTargetCommand.py` would 
only run if `en-US` is the current locale. Maybe afterwards, in a subsequent 
patch, we could add more code to temporarily switch to `en-US` locale if it's 
available?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88975

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


[Lldb-commits] [PATCH] D88939: [lldb] Remove unused code in GetVersion (NFC)

2020-10-07 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

Yeah there isn't a lot of unused code left here. If you want you can make a 
follow up replacing all the C-string logic with `std::string` (and removing the 
static local result var as that seems unnecessary).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88939

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


[Lldb-commits] [PATCH] D88975: [LLDB] On Windows, fix tests

2020-10-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I really want to avoid having language specific strings in the tests. Skipping 
tests which depend on these is a reasonable approach (as you've discovered, 
there shouldn't be that many of them -- it's really just the tests which check 
that we actually can retrieve the error from the OS), and it could be later 
improved by switching to an english locale on a best-effort basis (i.e., if the 
locale is available).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88975

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


[Lldb-commits] [PATCH] D88975: [LLDB] On Windows, fix tests

2020-10-07 Thread Alexandre Ganea via Phabricator via lldb-commits
aganea added a comment.

In D88975#2317566 , @labath wrote:

> I really want to avoid having language specific strings in the tests.

We all agree on that. I'll update the patch once it passes all tests on my 
machines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88975

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


[Lldb-commits] [PATCH] D88967: [lldb] Add a cmake warning about the python/swig incompatibility

2020-10-07 Thread Adrian McCarthy via Phabricator via lldb-commits
amccarth added a comment.

In D88967#2317545 , @labath wrote:

> In D88967#2317522 , @amccarth wrote:
>
>> If I recall correctly, the non-debug builds still had the problem, they just 
>> didn't have the assertion that made it obvious.
>
> Is that problem only theoretical (like, "you shouldn't be doing that") or 
> does it have some practical consequences (crashes, incorrect operation, etc.)?

My memory isn't that detailed.  I think it was an argument being passed in as a 
single value but being referenced as though it was an array or list.  Or vice 
versa.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88967

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


[Lldb-commits] [PATCH] D88992: [lldb] Fix "frame var" for large bitfields

2020-10-07 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.

LGTM I notice that we are using `m_byte_offset` directly a little above the 
line you fixed instead of `GetByteOffset()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88992

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


Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

2020-10-07 Thread Vitaly Buka via lldb-commits
It looks like all sanitizer builder are still offline
http://lab.llvm.org:8011/#/builders

On Tue, 6 Oct 2020 at 00:34, Galina Kistanova via cfe-commits <
cfe-comm...@lists.llvm.org> wrote:

> Hello everyone,
>
> The staging buildbot was up and running for 6 days now, and looks good.
>
> Tomorrow at 12:00 PM PDT we will switch the production buildbot to the new
> version.
>
> If you are a bot owner, you do not need to do anything at this point,
> unless I'll ask you to help.
> The buildbot will go down for a short period of time, and then a new
> version will come up and will accept connections from your bots.
>
> Please note that the new version has a bit different URL structure. You
> will need to update the bookmarks or scripts if you have stored direct URLs
> to inside the buldbot.
>
> We will be watching the production and staging bots and will be improving
> zorg for the next week or so.
>
> I will need your feedback about blame e-mails delivery, IRC reporting
> issues, and anything you could spot wrong with the new bot. I  hope the
> transition will go smoothly and we will handle issues quickly if any would
> come up.
>
> After production is good and we have about a week of running history, I'll
> ask the bot owners to upgrade buildbots on their side. Please do not
> upgrade your buildbots unless I'll ask you to. We are trying to limit a
> number of moving parts at this stage. We will start accepting change to
> zorg at this point. Please feel free to talk to me if you will have a
> special situation and if you would absolutely have to make changes.
>
> Thanks for your support and help. And please feel free to ask if you have
> questions.
>
> Galina
>
>
> On Thu, Sep 10, 2020 at 9:35 PM Galina Kistanova 
> wrote:
>
>> Hello everyone,
>>
>> The buildbot upgrade is entering the phase when the results to become
>> visible.
>>
>> No change is required at this time on any of the builders. The bot owners
>> could upgrade the buildbot on build computers later, at their convenience,
>> as this is not on the critical path.
>>
>> We are going to upgrade the staging bot first. Then, once that is stable
>> and all detected issues are resolved, we will upgrade the production bot.
>>
>> I will need some help with testing, and will be asking to move some of
>> the builders temporarily to the staging. LLVM buildbot is a piece of
>> critical infrastructure, so more eyes and hands in making sure it works
>> properly the better.
>>
>> I'll be posting updates and ETA of particular changes in this thread.
>>
>> Please feel free to ask if you have any questions or concerns.
>>
>> Thanks
>>
>> Galina
>>
>> ___
> cfe-commits mailing list
> cfe-comm...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 81b11c9 - Fix a macOS build break caused by 3dfb94986170.

2020-10-07 Thread Jim Ingham via lldb-commits

Author: Jim Ingham
Date: 2020-10-07T15:01:27-07:00
New Revision: 81b11c91070f3a969b64b2c2e6011b02450fa75f

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

LOG: Fix a macOS build break caused by 3dfb94986170.

Added: 


Modified: 

lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm

Removed: 




diff  --git 
a/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
 
b/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
index cfd44f9ae5ce..92bf716599b0 100644
--- 
a/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ 
b/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -415,8 +415,9 @@ static Status HandleFileAction(ProcessLaunchInfo 
&launch_info,
   secondary_fd =
   launch_info.GetPTY().OpenSecondary(O_RDWR, nullptr, 0);
 if (secondary_fd == PseudoTerminal::invalid_fd) {
+  std::string secondary_path = secondary_spec.GetPath();
   error.SetErrorStringWithFormat(
-  "unable to open secondary pty '%s'", secondary_path);
+  "unable to open secondary pty '%s'", secondary_path.c_str());
   return error; // Failure
 }
 [options setValue:[NSNumber numberWithInteger:secondary_fd]



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


[Lldb-commits] [PATCH] D88975: [LLDB] On Windows, fix tests

2020-10-07 Thread Alexandre Ganea via Phabricator via lldb-commits
aganea updated this revision to Diff 296809.
aganea marked an inline comment as done.
aganea added a comment.

As discussed above:

- Remove locale-specific messages.
- Skip relevant locale-sensitive TestTargetCommand tests.
- Only skip EXPECT calls in Utility/StatusTest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88975

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test_event/build_exception.py
  lldb/test/API/commands/target/basic/TestTargetCommand.py
  lldb/unittests/Utility/StatusTest.cpp

Index: lldb/unittests/Utility/StatusTest.cpp
===
--- lldb/unittests/Utility/StatusTest.cpp
+++ lldb/unittests/Utility/StatusTest.cpp
@@ -10,7 +10,7 @@
 #include "gtest/gtest.h"
 
 #ifdef _WIN32
-#include 
+#include 
 #endif
 
 using namespace lldb_private;
@@ -71,14 +71,26 @@
   EXPECT_FALSE(success.ToError());
   EXPECT_TRUE(success.Success());
 
+  WCHAR name[128]{};
+  ULONG langs = 0;
+  ULONG nameLen = sizeof(name) / sizeof(WCHAR);
+  GetUserPreferredUILanguages(MUI_LANGUAGE_NAME, &langs, (PZZWSTR)&name,
+  &nameLen);
+  // Skip the following tests on non-English, non-US, locales because the
+  // formatted messages will be different.
+  bool skip = wcscmp(L"en-US", name) != 0;
+
   auto s = Status(ERROR_ACCESS_DENIED, ErrorType::eErrorTypeWin32);
   EXPECT_TRUE(s.Fail());
-  EXPECT_STREQ("Access is denied. ", s.AsCString());
+  if (!skip)
+EXPECT_STREQ("Access is denied. ", s.AsCString());
 
   s.SetError(ERROR_IPSEC_IKE_TIMED_OUT, ErrorType::eErrorTypeWin32);
-  EXPECT_STREQ("Negotiation timed out ", s.AsCString());
+  if (!skip)
+EXPECT_STREQ("Negotiation timed out ", s.AsCString());
 
   s.SetError(16000, ErrorType::eErrorTypeWin32);
-  EXPECT_STREQ("unknown error", s.AsCString());
+  if (!skip)
+EXPECT_STREQ("unknown error", s.AsCString());
 }
 #endif
Index: lldb/test/API/commands/target/basic/TestTargetCommand.py
===
--- lldb/test/API/commands/target/basic/TestTargetCommand.py
+++ lldb/test/API/commands/target/basic/TestTargetCommand.py
@@ -350,6 +350,7 @@
 self.expect("target create a b", error=True,
 substrs=["'target create' takes exactly one executable path"])
 
+@skipIfWindowsAndNonEnglish
 @no_debug_info_test
 def test_target_create_nonexistent_core_file(self):
 self.expect("target create -c doesntexist", error=True,
@@ -365,6 +366,7 @@
 self.expect("target create -c '" + tf.name + "'", error=True,
 substrs=["Cannot open '", "': Permission denied"])
 
+@skipIfWindowsAndNonEnglish
 @no_debug_info_test
 def test_target_create_nonexistent_sym_file(self):
 self.expect("target create -s doesntexist doesntexisteither", error=True,
Index: lldb/packages/Python/lldbsuite/test_event/build_exception.py
===
--- lldb/packages/Python/lldbsuite/test_event/build_exception.py
+++ lldb/packages/Python/lldbsuite/test_event/build_exception.py
@@ -12,4 +12,4 @@
 @staticmethod
 def format_build_error(command, command_output):
 return "Error when building test subject.\n\nBuild Command:\n{}\n\nBuild Command Output:\n{}".format(
-command, command_output.decode("utf-8"))
+command, command_output.decode("utf-8", errors='ignore'))
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -52,6 +52,9 @@
 """Returns true if fpath is an executable."""
 if fpath == None:
 return False
+if sys.platform == 'win32':
+if not fpath.endswith(".exe"):
+fpath += ".exe"
 return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
 
 
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -3,6 +3,8 @@
 # System modules
 from distutils.version import LooseVersion
 from functools import wraps
+import ctypes
+import locale
 import os
 import platform
 import re
@@ -592,6 +594,17 @@
 """Decorate the item to skip tests that should be skipped on Windows."""
 return skipIfPlatform(["windows"])(func)
 
+def skipIfWindowsAndNonEnglish(func):
+"""Decorate the item to skip tests that should be skipped on non-English locales on Windows."""
+def is_Windows_NonEnglish(self):
+if lldbplatformutil.getPlatform() != "windows":
+return None
+kernel = ctypes.windll.kernel32
+if locale.

[Lldb-commits] [PATCH] D82863: [LLDB] Add support to resize SVE registers at run-time

2020-10-07 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid updated this revision to Diff 296811.
omjavaid added a comment.

This is an update after addressing comments from @jasonmolenda


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

https://reviews.llvm.org/D82863

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -38,6 +38,8 @@
 
 #include "llvm/ADT/DenseMap.h"
 
+#include "Utility/ARM64_DWARF_Registers.h"
+
 namespace lldb_private {
 namespace repro {
 class Loader;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1761,6 +1761,27 @@
 gdb_thread->PrivateSetRegisterValue(pair.first, buffer_sp->GetData());
   }
 
+  // Code below is specific to AArch64 target in SVE state
+  // If expedited register set contains vector granule (vg) register
+  // then thread's register context reconfiguration is triggered by
+  // calling UpdateARM64SVERegistersInfos.
+  const ArchSpec &arch = GetTarget().GetArchitecture();
+  if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64 ||
+ arch.GetMachine() == llvm::Triple::aarch64_be)) {
+uint8_t arm64_sve_vg_dwarf_regnum = arm64_dwarf::vg;
+GDBRemoteRegisterContext *reg_ctx_sp =
+static_cast(
+gdb_thread->GetRegisterContext().get());
+
+if (reg_ctx_sp) {
+  uint32_t vg_regnum = reg_ctx_sp->ConvertRegisterKindToRegisterNumber(
+  eRegisterKindDWARF, arm64_sve_vg_dwarf_regnum);
+  if (expedited_register_map.count(vg_regnum)) {
+reg_ctx_sp->AArch64SVEReconfigure();
+  }
+}
+  }
+
   thread_sp->SetName(thread_name.empty() ? nullptr : thread_name.c_str());
 
   gdb_thread->SetThreadDispatchQAddr(thread_dispatch_qaddr);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -40,6 +40,8 @@
 
   void HardcodeARMRegisters(bool from_scratch);
 
+  bool UpdateARM64SVERegistersInfos(uint64_t vg, uint32_t &end_reg_offset);
+
   void CloneFrom(GDBRemoteDynamicRegisterInfoSP process_reginfo);
 };
 
@@ -79,6 +81,8 @@
   uint32_t ConvertRegisterKindToRegisterNumber(lldb::RegisterKind kind,
uint32_t num) override;
 
+  bool AArch64SVEReconfigure();
+
 protected:
   friend class ThreadGDBRemote;
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -343,6 +343,17 @@
   if (dst == nullptr)
 return false;
 
+  // Code below is specific to AArch64 target in SVE state
+  // If vector granule (vg) register is being written then thread's
+  // register context reconfiguration is triggered on success.
+  bool do_reconfigure_arm64_sve = false;
+  const ArchSpec &arch = process->GetTarget().GetArchitecture();
+  if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64 ||
+ arch.GetMachine() == llvm::Triple::aarch64_be)) {
+if (reg_info->kinds[eRegisterKindDWARF] == arm64_dwarf::vg)
+  do_reconfigure_arm64_sve = true;
+  }
+
   if (data.CopyByteOrderedData(data_offset,// src offset
reg_info->byte_size,// src length
dst,// dst
@@ -362,6 +373,11 @@
 
 {
   SetAllRegisterValid(false);
+
+  if (do_reconfigure_arm64_sve &&
+  GetPrimordialRegister(reg_info, gdb_comm))
+AArch64SVEReconfigure();
+
   return true;
 }
   } else {
@@ -390,6 +406,10 @@
 } else {
   // This is an actual register, write it
   success = SetPrimordialRegister(reg_info, gdb_comm);
+
+  if (success && do_reconfigure_arm64_sve &&
+  GetPrimordialRegister(reg_info, gdb_comm))
+AAr

[Lldb-commits] [PATCH] D82863: [LLDB] Add support to resize SVE registers at run-time

2020-10-07 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp:767
+  m_reg_data.Clear();
+  m_reg_data.SetData(reg_data_sp);
+  m_reg_data.SetByteOrder(GetByteOrder());

jasonmolenda wrote:
> I'm probably not following this correctly, but isn't this going to shorten 
> the register buffer m_reg_data to the end of the SVE registers in the buffer, 
> right?  If the goal here is to create a heap object, why not just copy the 
> entire m_data_reg?  If someone ever adds a register past the SVE's, this 
> would truncate it away.  
Register data can expand on shrink based on vector length update of SVE 
register set. So heap size will change accordingly.
We need to preserve VG register and GPRs from current register data so we copy 
the whole data in case vector length update caused register data to expand. 
However we truncate data to new vector length in case vector length has shrunk. 
All register past VG registers are invalidated anyways. 


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

https://reviews.llvm.org/D82863

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


[Lldb-commits] [PATCH] D88975: [LLDB] On Windows, fix tests

2020-10-07 Thread Alexandre Ganea via Phabricator via lldb-commits
aganea added a comment.

In D88975#2317494 , @amccarth wrote:

> It's interesting that I haven't encountered some of these errors.  There are 
> five _other_ lldb tests that do fail for me.  I have a fix in the works for 
> some of those.

Please see my failing tests (with VS2019 16.7.5):
F13201512: failing_lldb_vs2019.txt 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88975

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


[Lldb-commits] [PATCH] D88387: Create "skinny corefiles" for Mach-O with process save-core / reading

2020-10-07 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

I was thinking about the UI of this some more today.

I think process save-core should take a --core-style argument with an enum of 
(currently) 'full' or 'modified-memory'.  Plugin::SaveCore will take a 
requested_coredump_style and will return an created_coredump_style.  The user 
can request a modified-memory coredump on linux, but it's not supported so they 
get a full coredump.  We could have "stack-only" coredumps (this would be an 
awful lot like a minidump iiuc, and may be duplicative), or 
"modified-memory-plus-binary-images" (better name tbd) which would copy in the 
binary images of any executing binaries or whatever.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88387

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


[Lldb-commits] [PATCH] D88967: [lldb] Add a cmake warning about the python/swig incompatibility

2020-10-07 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.

Neat!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88967

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


Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

2020-10-07 Thread Galina Kistanova via lldb-commits
They are online now - http://lab.llvm.org:8011/#/waterfall?tags=sanitizer

AnnotatedCommand has severe design conflict with the new buildbot.
We have changed it to be safe and still do something useful, but it will
need more love and care.

Please let me know if you have some spare time to work on porting
AnnotatedCommand.

Thanks

Galina

On Wed, Oct 7, 2020 at 2:57 PM Vitaly Buka  wrote:

> It looks like all sanitizer builder are still offline
> http://lab.llvm.org:8011/#/builders
>
> On Tue, 6 Oct 2020 at 00:34, Galina Kistanova via cfe-commits <
> cfe-comm...@lists.llvm.org> wrote:
>
>> Hello everyone,
>>
>> The staging buildbot was up and running for 6 days now, and looks good.
>>
>> Tomorrow at 12:00 PM PDT we will switch the production buildbot to the
>> new version.
>>
>> If you are a bot owner, you do not need to do anything at this point,
>> unless I'll ask you to help.
>> The buildbot will go down for a short period of time, and then a new
>> version will come up and will accept connections from your bots.
>>
>> Please note that the new version has a bit different URL structure. You
>> will need to update the bookmarks or scripts if you have stored direct URLs
>> to inside the buldbot.
>>
>> We will be watching the production and staging bots and will be improving
>> zorg for the next week or so.
>>
>> I will need your feedback about blame e-mails delivery, IRC reporting
>> issues, and anything you could spot wrong with the new bot. I  hope the
>> transition will go smoothly and we will handle issues quickly if any would
>> come up.
>>
>> After production is good and we have about a week of running history,
>> I'll ask the bot owners to upgrade buildbots on their side. Please do
>> not upgrade your buildbots unless I'll ask you to. We are trying to
>> limit a number of moving parts at this stage. We will start accepting
>> change to zorg at this point. Please feel free to talk to me if you will
>> have a special situation and if you would absolutely have to make changes.
>>
>> Thanks for your support and help. And please feel free to ask if you have
>> questions.
>>
>> Galina
>>
>>
>> On Thu, Sep 10, 2020 at 9:35 PM Galina Kistanova 
>> wrote:
>>
>>> Hello everyone,
>>>
>>> The buildbot upgrade is entering the phase when the results to become
>>> visible.
>>>
>>> No change is required at this time on any of the builders. The bot
>>> owners could upgrade the buildbot on build computers later, at their
>>> convenience, as this is not on the critical path.
>>>
>>> We are going to upgrade the staging bot first. Then, once that is stable
>>> and all detected issues are resolved, we will upgrade the production bot.
>>>
>>> I will need some help with testing, and will be asking to move some of
>>> the builders temporarily to the staging. LLVM buildbot is a piece of
>>> critical infrastructure, so more eyes and hands in making sure it works
>>> properly the better.
>>>
>>> I'll be posting updates and ETA of particular changes in this thread.
>>>
>>> Please feel free to ask if you have any questions or concerns.
>>>
>>> Thanks
>>>
>>> Galina
>>>
>>> ___
>> cfe-commits mailing list
>> cfe-comm...@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D89019: Change the default handling of SIGCONT to nostop/noprint

2020-10-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision.
jingham added reviewers: labath, JDevlieghere, krytarowski, emaste.
Herald added subscribers: lldb-commits, atanasyan, jrtc27, sdardis.
Herald added a project: LLDB.
jingham requested review of this revision.

The macOS LaunchServices agent sends a SIGCONT to the process it is managing in 
the process of starting up.  This means processes that lldb attaches to when 
run by this agent stop somewhere in their startup with a SIGCONT.  That's a bit 
annoying if you are trying to debug, and really annoying if you are running 
batch tests in the debugger, for instance.

That can be fixed by changing your "SIGCONT" handling, but when the issue came 
to my attention, it got me wondering whether stopping in the debugger for 
SIGCONT is really useful behavior?  For the most part this means some job 
control operation was going on, but do you actually want to stop again after 
something, for instance, stopped & resumed your app?  It seems to me unless you 
are explicitly debugging an interaction with this job control, stopping is just 
an annoyance.

So I'm proposing to change the default behavior to nonstop/noprint/pass from 
stop/print/pass.

I'm adding folks that are responsible for some of the other Unixen platforms as 
reviewers.  If anybody has a good use case where this would be useful as a 
default, please speak up and I'll ditch this revision (and just get the clients 
that deal with LS to change the handling on their own.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89019

Files:
  lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
  lldb/source/Plugins/Process/Utility/MipsLinuxSignals.cpp
  lldb/source/Target/UnixSignals.cpp


Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -94,7 +94,7 @@
   AddSignal(16, "SIGURG", false,false,  false,  "urgent condition 
on IO channel");
   AddSignal(17, "SIGSTOP",true, true,   true,   "sendable stop 
signal not from tty");
   AddSignal(18, "SIGTSTP",false,true,   true,   "stop signal from 
tty");
-  AddSignal(19, "SIGCONT",false,true,   true,   "continue a 
stopped process");
+  AddSignal(19, "SIGCONT",false,false,  false,   "continue a 
stopped process");
   AddSignal(20, "SIGCHLD",false,false,  false,  "to parent on 
child stop or exit");
   AddSignal(21, "SIGTTIN",false,true,   true,   "to readers 
process group upon background tty read");
   AddSignal(22, "SIGTTOU",false,true,   true,   "to readers 
process group upon background tty write");
Index: lldb/source/Plugins/Process/Utility/MipsLinuxSignals.cpp
===
--- lldb/source/Plugins/Process/Utility/MipsLinuxSignals.cpp
+++ lldb/source/Plugins/Process/Utility/MipsLinuxSignals.cpp
@@ -45,7 +45,7 @@
 "SIGPOLL");
   AddSignal(23, "SIGSTOP", true, true, true, "process stop");
   AddSignal(24, "SIGTSTP", false, true, true, "tty stop");
-  AddSignal(25, "SIGCONT", false, true, true, "process continue");
+  AddSignal(25, "SIGCONT", false, false, false, "process continue");
   AddSignal(26, "SIGTTIN", false, true, true, "background tty read");
   AddSignal(27, "SIGTTOU", false, true, true, "background tty write");
   AddSignal(28, "SIGVTALRM", false, true, true, "virtual time alarm");
Index: lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
===
--- lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
+++ lldb/source/Plugins/Process/Utility/LinuxSignals.cpp
@@ -37,7 +37,7 @@
   AddSignal(16, "SIGSTKFLT", false, true, true, "stack fault");
   AddSignal(17, "SIGCHLD", false, false, true, "child status has changed",
 "SIGCLD");
-  AddSignal(18, "SIGCONT", false, true, true, "process continue");
+  AddSignal(18, "SIGCONT", false, false, false, "process continue");
   AddSignal(19, "SIGSTOP", true, true, true, "process stop");
   AddSignal(20, "SIGTSTP", false, true, true, "tty stop");
   AddSignal(21, "SIGTTIN", false, true, true, "background tty read");


Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -94,7 +94,7 @@
   AddSignal(16, "SIGURG", false,false,  false,  "urgent condition on IO channel");
   AddSignal(17, "SIGSTOP",true, true,   true,   "sendable stop signal not from tty");
   AddSignal(18, "SIGTSTP",false,true,   true,   "stop signal from tty");
-  AddSignal(19, "SIGCONT",false,true,   true,   "continue a stopped process");
+  AddSignal(19, "SIGCONT",false,false,  false,   "continue a stopped process");
   AddSignal(20, "SIGCHLD",false,false,  false,  "to parent on c

[Lldb-commits] [PATCH] D89019: Change the default handling of SIGCONT to nostop/noprint

2020-10-07 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

As discussed offline I also think this makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89019

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


[Lldb-commits] [PATCH] D88387: Create "skinny corefiles" for Mach-O with process save-core / reading

2020-10-07 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In D88387#2318226 , @jasonmolenda 
wrote:

> I was thinking about the UI of this some more today.
>
> I think process save-core should take a --core-style argument with an enum of 
> (currently) 'full' or 'modified-memory'.  Plugin::SaveCore will take a 
> requested_coredump_style and will return an created_coredump_style.  The user 
> can request a modified-memory coredump on linux, but it's not supported so 
> they get a full coredump.  We could have "stack-only" coredumps (this would 
> be an awful lot like a minidump iiuc, and may be duplicative), or 
> "modified-memory-plus-binary-images" (better name tbd) which would copy in 
> the binary images of any executing binaries or whatever.

That seems like a lot of flavors. At the command level I would suggest just 
some arguments. To the SaveCore, it can take a bitfield with all of the options 
the user selected. Options I can think of:

--full: which would specify to save all memory regions
--modified: save any modified data pages that are not thread stacks. I would 
suggest the core file plug-in returns an error if it doesn't support this to 
provide a better user experience. This could imply --stacks or it could be a 
stand alone option.
--stacks: save the thread stacks, very close to minidumps as you mentioned if 
only this is specified
--stack-functions: save memory for the code for all function bodies from all 
stack frames in all threads. This helps with stack tracing if we have no access 
to the system object files
--minimal-heap: save only the allocated memory from heaps. Often times the 
system malloc allocators will grab large chunks of memory and hand out partial 
chunks when malloc is called. On Darwin, we could easily find out the actual 
allocations that are live on the heap and only save 
those?
--crashing-thread-only: only save threads that crashed. Cuts down on memory 
size for stacks, and stack functions.

We could then make a bitfield of options:

  enum uint32_t CoreFileOptions {
eFull = (1u << 0),
eModifiedPages = (1u << 1),
eThreadStacks = (1u << 2),
eThreadFunctions = (1u << 3),
eMinimalHead = (1u << 4),
eCrashingThreadOnly = (1u << 5)
  };

And then pass a "uint32_t flags" to the save core instead of the bool.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88387

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


[Lldb-commits] [PATCH] D88387: Create "skinny corefiles" for Mach-O with process save-core / reading

2020-10-07 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

Yeah, I'm not opposed to mutually exclusive options to process save-core to 
request a specific style of coredump to be created, whether it's a --core-style 
enum or a series of flags isn't important to me.  Right now I only intend to 
add the two that are possible - a full coredump or a modified-memory (dirty 
memory) coredump, on macOS.  Whether it's passed as an enum or bitflag I don't 
have a preference, although a bitflag makes it sound like there could be 
multiple of them selected, when that's not what I'd be envisioning here.  (if 
we were doing a series of flags for the lldb API, it would make more sense if 
we had different types of memory that could be coredumped and you could set the 
flags for what you want included.  stack, heap, shared memory, binary images, 
etc.)

I forgot to respond to your suggestion about the SB API Process::SaveCore.  I'm 
a little reluctant to add anything there yet because we're going from one style 
of coredump to two right now, and I'm not sure if we're going to stay at 2 
forever or if we're going to evolve this into a panoply of different coredump 
styles and I'm not confident enough about this to want to put it in API.  But 
I'm not wedded to that position.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88387

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


[Lldb-commits] [PATCH] D88387: Create "skinny corefiles" for Mach-O with process save-core / reading

2020-10-07 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In D88387#2318495 , @jasonmolenda 
wrote:

> Yeah, I'm not opposed to mutually exclusive options to process save-core to 
> request a specific style of coredump to be created, whether it's a 
> --core-style enum or a series of flags isn't important to me.  Right now I 
> only intend to add the two that are possible - a full coredump or a 
> modified-memory (dirty memory) coredump, on macOS.  Whether it's passed as an 
> enum or bitflag I don't have a preference, although a bitflag makes it sound 
> like there could be multiple of them selected, when that's not what I'd be 
> envisioning here.  (if we were doing a series of flags for the lldb API, it 
> would make more sense if we had different types of memory that could be 
> coredumped and you could set the flags for what you want included.  stack, 
> heap, shared memory, binary images, etc.)

I think eventually it would be nice to get to multiple of the flags, but we can 
start with full or dirty only is fine.

> I forgot to respond to your suggestion about the SB API Process::SaveCore.  
> I'm a little reluctant to add anything there yet because we're going from one 
> style of coredump to two right now, and I'm not sure if we're going to stay 
> at 2 forever or if we're going to evolve this into a panoply of different 
> coredump styles and I'm not confident enough about this to want to put it in 
> API.  But I'm not wedded to that position.

Sounds good, lets work this out first internally and then expose via the SB API 
when we are ready.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88387

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


[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

2020-10-07 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 296866.
wallace edited the summary of this revision.
wallace added a comment.

Addressed comments, which includse:

- Implemented the requested repeat command logic, including tests

- Now showing the total number of instructions as part of the dump command, 
which is something like

  thread #1: tid = 3842849, total instructions = 1000 instruction 0 instruction 
1 instruction 2 ...

- Moved some logic to the parent diff D88841 , 
leaving this diff simpler.

Some notes:

- In my current design, TraceProcess will be shared by all trace plug-ins when 
loading json trace sessions. For live processes, we'll simply use the actual 
Process instance used by the process, which means that most of the trace logic 
regarding processes will end up being in the base Process.
- In my next diff I'll include the trace decoding functionality, which will 
remove the placeholder text printed when invoking the dump command.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88769

Files:
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Process/CMakeLists.txt
  lldb/source/Plugins/Process/Trace/CMakeLists.txt
  lldb/source/Plugins/Process/Trace/ProcessTrace.cpp
  lldb/source/Plugins/Process/Trace/ProcessTrace.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/Thread.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/intelpt-trace/trace_2threads.json

Index: lldb/test/API/commands/trace/intelpt-trace/trace_2threads.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_2threads.json
@@ -0,0 +1,35 @@
+{
+  "trace": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "processes": [
+{
+  "pid": 1234,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 3842849,
+  "traceFile": "3842849.trace"
+},
+{
+  "tid": 3842850,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+}
+  ]
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -35,6 +35,10 @@
 
 self.assertEqual("6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A", module.GetUUIDString())
 
+# check that the Process and Thread objects were created correctly
+self.expect("thread info", substrs=["tid = 3842849"])
+self.expect("thread list", substrs=["Process 1234 stopped", "tid = 3842849"])
+
 
 def testLoadInvalidTraces(self):
 src_dir = self.getSourceDir()
Index: lldb/test/API/commands/trace/TestTraceDumpInstructions.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceDumpInstructions.py
@@ -0,0 +1,91 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceDumpInstructions(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+TestBase.setUp(self)
+if 'intel-pt' not in configuration.enabled_plugins:
+self.skipTest("The intel-pt test plugin is not enabled")
+
+def testErrorMessages(self):
+# We first check the output when there are no targets
+self.expect("thread trace dump instructions",
+substrs=["error: invalid target, create a target using the 'target create' command"],
+error=True)
+
+# We now check the output when there's a non-running target
+self.expect("target create " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+
+self.expect("thread trace dump instructions",
+substrs=["error: invalid process"],
+error=True)
+
+# Now we check the output when there's a running target without a trace
+self.expect("b main")
+self.expect("run")
+
+self.expect("thread trace dump instructions",
+substrs=["error: no trace in this target"])
+
+def testDumpInstructions(self):
+self.expect("trace load -v " + os.