[Lldb-commits] [PATCH] D79614: Fix error reporting for qLaunchSuccess, check for fix/enable it via qSupported

2020-05-07 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda created this revision.
jasonmolenda added reviewers: labath, clayborg.
jasonmolenda added a project: LLDB.

I've been experimenting with returning richer error messages in debugserver 
recently, and I found a bug the other day where qLaunchSuccess returns its 
error code with an "E" followed by the text of the error message.  If the error 
message contains one of the magic gdb RSP characters (#, $, *, }), lldb will 
often crash.  We've seen packets like vAttach adopt a new method of returning 
an error message as Exx;HEX-ENCODED instead of the usual Exx.

We've had instances where packets need to be fixed in the past, e.g. Spencer 
Michaels reports a bug in the A packet here 
https://bugs.llvm.org/show_bug.cgi?id=42471 and I need to fix the constant 
values we use in the vFile packets.  Spender and I were discussing a 
"qProtocolFixes" packet, but Fred suggested we could piggyback on qSupported.  
I liked that idea, so that's what I have implemented here.

lldb will send qLaunchSuccessASCIIHexErrorText in its qSupported request, and 
if this is a debugserver that has the fix, it will enable the corrected error 
reporting mode and include qLaunchSuccessASCIIHexErrorText in the qSupported 
response.  I think this is a pretty clean way of fixing things like this, 
Greg/Pavel what do you think?

I also found a bug in debugserver's cstring_to_asciihex_string() function with 
high-bit set characters in the error string (e.g. a utf-8 character) with 
signed chars where we'd end up with a bunch of 0xff's in the ascii hex string.

I'm trying to make an API test for this but haven't found a good way to fake up 
enough of the lldb side to add a gdb_remote_client test yet.

rdar://problem/62873581


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79614

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/tools/debugserver/source/RNBRemote.cpp
  lldb/tools/debugserver/source/RNBRemote.h

Index: lldb/tools/debugserver/source/RNBRemote.h
===
--- lldb/tools/debugserver/source/RNBRemote.h
+++ lldb/tools/debugserver/source/RNBRemote.h
@@ -405,6 +405,7 @@
   size_t m_compression_minsize; // only packets larger than this size will be
 // compressed
   bool m_enable_compression_next_send_packet;
+  bool m_qLaunchSuccess_error_fix;
 
   compression_types m_compression_mode;
 };
Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -145,6 +145,18 @@
   return addr;
 }
 
+std::string cstring_to_asciihex_string(const char *str) {
+  std::string hex_str;
+  hex_str.reserve(strlen(str) * 2);
+  while (str && *str) {
+uint8_t c = *str++;
+char hexbuf[5];
+snprintf(hexbuf, sizeof(hexbuf), "%02x", c);
+hex_str += hexbuf;
+  }
+  return hex_str;
+}
+
 extern void ASLLogCallback(void *baton, uint32_t flags, const char *format,
va_list args);
 
@@ -176,6 +188,7 @@
   m_extended_mode(false), m_noack_mode(false),
   m_thread_suffix_supported(false), m_list_threads_in_stop_reply(false),
   m_compression_minsize(384), m_enable_compression_next_send_packet(false),
+  m_qLaunchSuccess_error_fix(false),
   m_compression_mode(compression_types::none) {
   DNBLogThreadedIf(LOG_RNB_REMOTE, "%s", __PRETTY_FUNCTION__);
   CreatePacketTable();
@@ -1643,7 +1656,13 @@
 return SendPacket("OK");
   std::ostringstream ret_str;
   std::string status_str;
-  ret_str << "E" << m_ctx.LaunchStatusAsString(status_str);
+  if (m_qLaunchSuccess_error_fix) {
+std::string hexified =
+cstring_to_asciihex_string(m_ctx.LaunchStatusAsString(status_str));
+ret_str << "E97;" << hexified.c_str();
+  } else {
+ret_str << "E" << m_ctx.LaunchStatusAsString(status_str);
+  }
 
   return SendPacket(ret_str.str());
 }
@@ -2673,17 +2692,6 @@
   }
 }
 
-std::string cstring_to_asciihex_string(const char *str) {
-  std::string hex_str;
-  hex_str.reserve (strlen (str) * 2);
-  while (str && *str) {
-char hexbuf[5];
-snprintf (hexbuf, sizeof(hexbuf), "%02x", *str++);
-hex_str += hexbuf;
-  }
-  return hex_str;
-}
-
 void append_hexified_string(std::ostream , const std::string ) {
   size_t string_size = string.size();
   const char *string_buf = string.c_str();
@@ -3647,7 +3655,13 @@
 snprintf(numbuf, sizeof(numbuf), "%zu", m_compression_minsize);
 numbuf[sizeof(numbuf) - 1] = '\0';
 strcat(buf, numbuf);
-  } 
+  }
+
+  // Enable a fix to the qLaunchSuccess packet.
+  if (strstr(p, "qLaunchSuccessASCIIHexErrorText") != nullptr) {
+m_qLaunchSuccess_error_fix = true;
+strcat(buf, ";qLaunchSuccessASCIIHexErrorText");
+  }
 
   return SendPacket(buf);
 }
Index: 

[Lldb-commits] [lldb] 2ea7187 - Add a new lockdownd plist for launching posix processes

2020-05-07 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2020-05-07T18:53:51-07:00
New Revision: 2ea7187ab9b7f0eab38a0b5be45c48b1f4f4938d

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

LOG: Add a new lockdownd plist for launching posix processes

Similar to
com.apple.debugserver.plist & com.apple.debugserver.internal.plist
com.apple.debugserver.applist.plist & 
com.apple.debugserver.applist.internal.plist
add a variant of the posix plist.



Added: 
lldb/tools/debugserver/source/com.apple.debugserver.posix.internal.plist

Modified: 


Removed: 




diff  --git 
a/lldb/tools/debugserver/source/com.apple.debugserver.posix.internal.plist 
b/lldb/tools/debugserver/source/com.apple.debugserver.posix.internal.plist
new file mode 100644
index ..0ca9f5be68e2
--- /dev/null
+++ b/lldb/tools/debugserver/source/com.apple.debugserver.posix.internal.plist
@@ -0,0 +1,16 @@
+
+http://www.apple.com/DTDs/PropertyList-1.0.dtd;>
+
+
+   Label
+   com.apple.debugserver.posix.internal
+   ProgramArguments
+   
+   /Developer/usr/bin/debugserver
+   --lockdown
+   --launch=posix
+   
+AllowByProxy
+
+
+



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


[Lldb-commits] [PATCH] D79607: [lldb/test][Darwin] Ask dyld where the real python is

2020-05-07 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 262799.
vsk added a comment.

- Reinstated comment about SIP to explain why we copy python
- Added a platform check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79607

Files:
  lldb/test/API/get_darwin_real_python.py
  lldb/test/API/lldbtest.py


Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -3,6 +3,7 @@
 import tempfile
 import subprocess
 import sys
+import platform
 
 import lit.Test
 import lit.TestRunner
@@ -75,19 +76,21 @@
 builddir = getBuildDir(cmd)
 mkdir_p(builddir)
 
-# The macOS system integrity protection (SIP) doesn't allow injecting
-# libraries into system binaries, but this can be worked around by
-# copying the binary into a different location.
+# On macOS, we can't do the DYLD_INSERT_LIBRARIES trick with a shim
+# python binary as the ASan interceptors get loaded too late. Also,
+# when SIP is enabled, we can't inject libraries into system binaries
+# at all, so we need a copy of the "real" python to work with.
+#
+# Find the "real" python binary, copy it, and invoke it.
 if 'DYLD_INSERT_LIBRARIES' in test.config.environment and \
-(executable.startswith('/System/') or \
-executable.startswith('/usr/bin/')):
+platform.system() == 'Darwin':
 copied_python = os.path.join(builddir, 'copied-system-python')
 if not os.path.isfile(copied_python):
 import shutil, subprocess
 python = subprocess.check_output([
 executable,
-'-c',
-'import sys; print(sys.executable)'
+os.path.join(os.path.dirname(os.path.realpath(__file__)),
+'get_darwin_real_python.py')
 ]).decode('utf-8').strip()
 shutil.copy(python, copied_python)
 cmd[0] = copied_python
Index: lldb/test/API/get_darwin_real_python.py
===
--- /dev/null
+++ lldb/test/API/get_darwin_real_python.py
@@ -0,0 +1,14 @@
+# On macOS, system python binaries like /usr/bin/python and $(xcrun -f python3)
+# are shims. They do some light validation work and then spawn the "real" 
python
+# binary. Find the "real" python by asking dyld -- sys.executable reports the
+# wrong thing more often than not. This is also useful when we're running under
+# a Homebrew python3 binary, which also appears to be some kind of shim.
+def getDarwinRealPythonExecutable():
+import ctypes
+dyld = ctypes.cdll.LoadLibrary('/usr/lib/system/libdyld.dylib')
+namelen = ctypes.c_ulong(1024)
+name = ctypes.create_string_buffer(b'\000', namelen.value)
+dyld._NSGetExecutablePath(ctypes.byref(name), ctypes.byref(namelen))
+return name.value.decode('utf-8').strip()
+
+print(getDarwinRealPythonExecutable())


Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -3,6 +3,7 @@
 import tempfile
 import subprocess
 import sys
+import platform
 
 import lit.Test
 import lit.TestRunner
@@ -75,19 +76,21 @@
 builddir = getBuildDir(cmd)
 mkdir_p(builddir)
 
-# The macOS system integrity protection (SIP) doesn't allow injecting
-# libraries into system binaries, but this can be worked around by
-# copying the binary into a different location.
+# On macOS, we can't do the DYLD_INSERT_LIBRARIES trick with a shim
+# python binary as the ASan interceptors get loaded too late. Also,
+# when SIP is enabled, we can't inject libraries into system binaries
+# at all, so we need a copy of the "real" python to work with.
+#
+# Find the "real" python binary, copy it, and invoke it.
 if 'DYLD_INSERT_LIBRARIES' in test.config.environment and \
-(executable.startswith('/System/') or \
-executable.startswith('/usr/bin/')):
+platform.system() == 'Darwin':
 copied_python = os.path.join(builddir, 'copied-system-python')
 if not os.path.isfile(copied_python):
 import shutil, subprocess
 python = subprocess.check_output([
 executable,
-'-c',
-'import sys; print(sys.executable)'
+os.path.join(os.path.dirname(os.path.realpath(__file__)),
+'get_darwin_real_python.py')
 ]).decode('utf-8').strip()
 shutil.copy(python, copied_python)
 cmd[0] = copied_python
Index: lldb/test/API/get_darwin_real_python.py

[Lldb-commits] [PATCH] D79607: [lldb/test][Darwin] Ask dyld where the real python is

2020-05-07 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/test/API/lldbtest.py:78
 
-# The macOS system integrity protection (SIP) doesn't allow injecting
-# libraries into system binaries, but this can be worked around by
-# copying the binary into a different location.
-if 'DYLD_INSERT_LIBRARIES' in test.config.environment and \
-(executable.startswith('/System/') or \
-executable.startswith('/usr/bin/')):
+# On macOS, we can't do the DYLD_INSERT_LIBRARIES trick with a shim
+# python binary as the ASan interceptors get loaded too late. Find the

SIP still matters here, which is why we need to copy the python binary and not 
just invoke it with the result from get_darwin_real_python.py. Can you merge 
the two comments?

I also think that we might set DYLD_INSERT_LIBRARIES unconditionally on other 
hosts. Can we guard this whole thing with a check for macOS?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79607



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


[Lldb-commits] [PATCH] D79607: [lldb/test][Darwin] Ask dyld where the real python is

2020-05-07 Thread Vedant Kumar via Phabricator via lldb-commits
vsk updated this revision to Diff 262792.
vsk added a comment.

Query dyld while running within the right python process.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79607

Files:
  lldb/test/API/get_darwin_real_python.py
  lldb/test/API/lldbtest.py


Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -75,19 +75,17 @@
 builddir = getBuildDir(cmd)
 mkdir_p(builddir)
 
-# The macOS system integrity protection (SIP) doesn't allow injecting
-# libraries into system binaries, but this can be worked around by
-# copying the binary into a different location.
-if 'DYLD_INSERT_LIBRARIES' in test.config.environment and \
-(executable.startswith('/System/') or \
-executable.startswith('/usr/bin/')):
+# On macOS, we can't do the DYLD_INSERT_LIBRARIES trick with a shim
+# python binary as the ASan interceptors get loaded too late. Find the
+# "real" python binary, copy it, and invoke it.
+if 'DYLD_INSERT_LIBRARIES' in test.config.environment:
 copied_python = os.path.join(builddir, 'copied-system-python')
 if not os.path.isfile(copied_python):
 import shutil, subprocess
 python = subprocess.check_output([
 executable,
-'-c',
-'import sys; print(sys.executable)'
+os.path.join(os.path.dirname(os.path.realpath(__file__)),
+'get_darwin_real_python.py')
 ]).decode('utf-8').strip()
 shutil.copy(python, copied_python)
 cmd[0] = copied_python
Index: lldb/test/API/get_darwin_real_python.py
===
--- /dev/null
+++ lldb/test/API/get_darwin_real_python.py
@@ -0,0 +1,14 @@
+# On macOS, system python binaries like /usr/bin/python and $(xcrun -f python3)
+# are shims. They do some light validation work and then spawn the "real" 
python
+# binary. Find the "real" python by asking dyld -- sys.executable reports the
+# wrong thing more often than not. This is also useful when we're running under
+# a Homebrew python3 binary, which also appears to be some kind of shim.
+def getDarwinRealPythonExecutable():
+import ctypes
+dyld = ctypes.cdll.LoadLibrary('/usr/lib/system/libdyld.dylib')
+namelen = ctypes.c_ulong(1024)
+name = ctypes.create_string_buffer(b'\000', namelen.value)
+dyld._NSGetExecutablePath(ctypes.byref(name), ctypes.byref(namelen))
+return name.value.decode('utf-8').strip()
+
+print(getDarwinRealPythonExecutable())


Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -75,19 +75,17 @@
 builddir = getBuildDir(cmd)
 mkdir_p(builddir)
 
-# The macOS system integrity protection (SIP) doesn't allow injecting
-# libraries into system binaries, but this can be worked around by
-# copying the binary into a different location.
-if 'DYLD_INSERT_LIBRARIES' in test.config.environment and \
-(executable.startswith('/System/') or \
-executable.startswith('/usr/bin/')):
+# On macOS, we can't do the DYLD_INSERT_LIBRARIES trick with a shim
+# python binary as the ASan interceptors get loaded too late. Find the
+# "real" python binary, copy it, and invoke it.
+if 'DYLD_INSERT_LIBRARIES' in test.config.environment:
 copied_python = os.path.join(builddir, 'copied-system-python')
 if not os.path.isfile(copied_python):
 import shutil, subprocess
 python = subprocess.check_output([
 executable,
-'-c',
-'import sys; print(sys.executable)'
+os.path.join(os.path.dirname(os.path.realpath(__file__)),
+'get_darwin_real_python.py')
 ]).decode('utf-8').strip()
 shutil.copy(python, copied_python)
 cmd[0] = copied_python
Index: lldb/test/API/get_darwin_real_python.py
===
--- /dev/null
+++ lldb/test/API/get_darwin_real_python.py
@@ -0,0 +1,14 @@
+# On macOS, system python binaries like /usr/bin/python and $(xcrun -f python3)
+# are shims. They do some light validation work and then spawn the "real" python
+# binary. Find the "real" python by asking dyld -- sys.executable reports the
+# wrong thing more often than not. This is also useful when we're running under
+# a Homebrew python3 binary, which also appears to be some kind of shim.
+def getDarwinRealPythonExecutable():
+import 

[Lldb-commits] [PATCH] D79607: [lldb/test][Darwin] Ask dyld where the real python is

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

Thanks Vedant for coming up with a structural solution to this problem!




Comment at: lldb/test/API/lldbtest.py:36
+# a Homebrew python3 binary, which also appears to be some kind of shim.
+def getDarwinRealPythonExecutable():
+import ctypes

This would have to run under the Python in `executable`, not the one lit is 
running with.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79607



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


[Lldb-commits] [PATCH] D79603: Add an API to construct an XcodeSDK from an SDK type.

2020-05-07 Thread Vedant Kumar via Phabricator via lldb-commits
vsk added inline comments.



Comment at: lldb/source/Utility/XcodeSDK.cpp:44
+  }
+  static_assert(XcodeSDK::Linux == XcodeSDK::numSDKTypes - 1,
+"New SDK type was added, update this list!");

I'd expect -Wcovered-switch to break the build if a new case gets added and 
this list isn't updated. Wdyt of dropping numSDKTypes and using an 
llvm_unreachable here instead?


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

https://reviews.llvm.org/D79603



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


[Lldb-commits] [PATCH] D79607: [lldb/test][Darwin] Ask dyld where the real python is

2020-05-07 Thread Vedant Kumar via Phabricator via lldb-commits
vsk created this revision.
vsk added a reviewer: JDevlieghere.
Herald added a project: LLDB.

On macOS, we can't do the DYLD_INSERT_LIBRARIES trick with a shim
python binary as the ASan interceptors get loaded too late. Find the
"real" python binary, copy it, and invoke it.

Hopefully this makes the GreenDragon and swift-ci sanitizer bots
happy...

I tested this out by running `../llvm-macosx-x86_64/bin/llvm-lit test
--filter TestNSDictionarySynthetic.py` in an ASanified swift-lldb build
directory and it worked (i.e. no more "interceptors loaded too late"
messages).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79607

Files:
  lldb/test/API/lldbtest.py


Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -28,6 +28,19 @@
 if not os.path.isdir(path):
 raise OSError(errno.ENOTDIR, "%s is not a directory"%path)
 
+# On macOS, system python binaries like /usr/bin/python and $(xcrun -f python3)
+# are shims. They do some light validation work and then spawn the "real" 
python
+# binary. Find the "real" python by asking dyld -- sys.executable reports the
+# wrong thing more often than not. This is also useful when we're running under
+# a Homebrew python3 binary, which also appears to be some kind of shim.
+def getDarwinRealPythonExecutable():
+import ctypes
+dyld = ctypes.cdll.LoadLibrary('/usr/lib/system/libdyld.dylib')
+namelen = ctypes.c_ulong(1024)
+name = ctypes.create_string_buffer(b'\000', namelen.value)
+dyld._NSGetExecutablePath(ctypes.byref(name), ctypes.byref(namelen))
+return name.value.decode('utf-8').strip()
+
 class LLDBTest(TestFormat):
 def __init__(self, dotest_cmd):
 self.dotest_cmd = dotest_cmd
@@ -75,20 +88,14 @@
 builddir = getBuildDir(cmd)
 mkdir_p(builddir)
 
-# The macOS system integrity protection (SIP) doesn't allow injecting
-# libraries into system binaries, but this can be worked around by
-# copying the binary into a different location.
-if 'DYLD_INSERT_LIBRARIES' in test.config.environment and \
-(executable.startswith('/System/') or \
-executable.startswith('/usr/bin/')):
+# On macOS, we can't do the DYLD_INSERT_LIBRARIES trick with a shim
+# python binary as the ASan interceptors get loaded too late. Find the
+# "real" python binary, copy it, and invoke it.
+if 'DYLD_INSERT_LIBRARIES' in test.config.environment:
 copied_python = os.path.join(builddir, 'copied-system-python')
 if not os.path.isfile(copied_python):
-import shutil, subprocess
-python = subprocess.check_output([
-executable,
-'-c',
-'import sys; print(sys.executable)'
-]).decode('utf-8').strip()
+import shutil
+python = getDarwinRealPythonExecutable()
 shutil.copy(python, copied_python)
 cmd[0] = copied_python
 


Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -28,6 +28,19 @@
 if not os.path.isdir(path):
 raise OSError(errno.ENOTDIR, "%s is not a directory"%path)
 
+# On macOS, system python binaries like /usr/bin/python and $(xcrun -f python3)
+# are shims. They do some light validation work and then spawn the "real" python
+# binary. Find the "real" python by asking dyld -- sys.executable reports the
+# wrong thing more often than not. This is also useful when we're running under
+# a Homebrew python3 binary, which also appears to be some kind of shim.
+def getDarwinRealPythonExecutable():
+import ctypes
+dyld = ctypes.cdll.LoadLibrary('/usr/lib/system/libdyld.dylib')
+namelen = ctypes.c_ulong(1024)
+name = ctypes.create_string_buffer(b'\000', namelen.value)
+dyld._NSGetExecutablePath(ctypes.byref(name), ctypes.byref(namelen))
+return name.value.decode('utf-8').strip()
+
 class LLDBTest(TestFormat):
 def __init__(self, dotest_cmd):
 self.dotest_cmd = dotest_cmd
@@ -75,20 +88,14 @@
 builddir = getBuildDir(cmd)
 mkdir_p(builddir)
 
-# The macOS system integrity protection (SIP) doesn't allow injecting
-# libraries into system binaries, but this can be worked around by
-# copying the binary into a different location.
-if 'DYLD_INSERT_LIBRARIES' in test.config.environment and \
-(executable.startswith('/System/') or \
-executable.startswith('/usr/bin/')):
+# On macOS, we can't do the DYLD_INSERT_LIBRARIES trick with a shim
+# python binary as the ASan interceptors get loaded too late. Find the
+# "real" python binary, copy it, and invoke it.
+if 'DYLD_INSERT_LIBRARIES' 

[Lldb-commits] [lldb] 13062d0 - [lldb/Test] Skip more tests that are not expected to work with passive replay

2020-05-07 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-05-07T15:16:52-07:00
New Revision: 13062d0fb76913802f94314090b1a2b36bd480d9

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

LOG: [lldb/Test] Skip more tests that are not expected to work with passive 
replay

This skips some tests that pass with active replay (which doesn't check
the output) but fail with passive replay. Valid reasons for this
include:

 - Checking the output of the process (which doesn't run during replay),
 - Checking files that cannot be captured in the VFS (non-existing or
   unreadable files or files that are removed during test),

Unfortunately there's no good way to mark a test as supported for active
replay but unsupported for passive replay because the number and order
of API calls needs to be identical during capture and replay. I don't
think this is a huge loss however.

Added: 


Modified: 
lldb/test/API/commands/process/launch/TestProcessLaunch.py
lldb/test/API/commands/target/basic/TestTargetCommand.py
lldb/test/API/python_api/process/io/TestProcessIO.py

Removed: 




diff  --git a/lldb/test/API/commands/process/launch/TestProcessLaunch.py 
b/lldb/test/API/commands/process/launch/TestProcessLaunch.py
index a2b27c2e5900..9e43c2c3fe28 100644
--- a/lldb/test/API/commands/process/launch/TestProcessLaunch.py
+++ b/lldb/test/API/commands/process/launch/TestProcessLaunch.py
@@ -29,6 +29,7 @@ def tearDown(self):
 TestBase.tearDown(self)
 
 @not_remote_testsuite_ready
+@skipIfReproducer
 def test_io(self):
 """Test that process launch I/O redirection flags work properly."""
 self.build()
@@ -84,6 +85,7 @@ def test_io(self):
 @not_remote_testsuite_ready
 @expectedFailureAll(oslist=["linux"], bugnumber="llvm.org/pr20265")
 @expectedFailureNetBSD
+@skipIfReproducer
 def test_set_working_dir_nonexisting(self):
 """Test that '-w dir' fails to set the working dir when running the 
inferior with a dir which doesn't exist."""
 d = {'CXX_SOURCES': 'print_cwd.cpp'}
@@ -111,6 +113,7 @@ def test_set_working_dir_nonexisting(self):
 invalid_dir_path])
 
 @not_remote_testsuite_ready
+@skipIfReproducer
 def test_set_working_dir_existing(self):
 """Test that '-w dir' sets the working dir when running the 
inferior."""
 d = {'CXX_SOURCES': 'print_cwd.cpp'}
@@ -170,6 +173,7 @@ def test_set_working_dir_existing(self):
 if not success:
 self.fail(err_msg)
 
+@skipIfReproducer
 def test_environment_with_special_char(self):
 """Test that environment variables containing '*' and '}' are handled 
correctly by the inferior."""
 source = 'print_env.cpp'

diff  --git a/lldb/test/API/commands/target/basic/TestTargetCommand.py 
b/lldb/test/API/commands/target/basic/TestTargetCommand.py
index 2704e0ed25ad..9bc9396e19ed 100644
--- a/lldb/test/API/commands/target/basic/TestTargetCommand.py
+++ b/lldb/test/API/commands/target/basic/TestTargetCommand.py
@@ -330,6 +330,7 @@ def test_target_create_nonexistent_core_file(self):
 
 # Write only files don't seem to be supported on Windows.
 @skipIfWindows
+@skipIfReproducer # Cannot be captured in the VFS.
 @no_debug_info_test
 def test_target_create_unreadable_core_file(self):
 tf = tempfile.NamedTemporaryFile()
@@ -353,6 +354,7 @@ def test_target_create_invalid_core_file(self):
 # Write only files don't seem to be supported on Windows.
 @skipIfWindows
 @no_debug_info_test
+@skipIfReproducer # Cannot be captured in the VFS.
 def test_target_create_unreadable_sym_file(self):
 tf = tempfile.NamedTemporaryFile()
 os.chmod(tf.name, stat.S_IWRITE)

diff  --git a/lldb/test/API/python_api/process/io/TestProcessIO.py 
b/lldb/test/API/python_api/process/io/TestProcessIO.py
index 365d486650f5..3c1c62d2d001 100644
--- a/lldb/test/API/python_api/process/io/TestProcessIO.py
+++ b/lldb/test/API/python_api/process/io/TestProcessIO.py
@@ -15,6 +15,7 @@ class ProcessIOTestCase(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfReproducer
 def setup_test(self):
 # Get the full path to our executable to be debugged.
 self.exe = self.getBuildArtifact("process_io")



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


[Lldb-commits] [PATCH] D79603: Add an API to construct an XcodeSDK from an SDK type.

2020-05-07 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision.
aprantl added reviewers: JDevlieghere, vsk.

Unfortunately I discovered that I do need to be able to go full round-trip in 
swift-lldb.

Also, this moves numSDKs out of the actual enum, as to not mess with the 
switch-cases-covered warning.


https://reviews.llvm.org/D79603

Files:
  lldb/include/lldb/Utility/XcodeSDK.h
  lldb/source/Utility/XcodeSDK.cpp
  lldb/unittests/Utility/XcodeSDKTest.cpp

Index: lldb/unittests/Utility/XcodeSDKTest.cpp
===
--- lldb/unittests/Utility/XcodeSDKTest.cpp
+++ lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -97,67 +97,82 @@
   EXPECT_FALSE(XcodeSDK("EverythingElse.sdk").SupportsSwift());
 }
 
-TEST(XcodeSDKTest, GetCanonicalName) {
+TEST(XcodeSDKTest, GetCanonicalNameAndConstruct) {
   XcodeSDK::Info info;
   info.type = XcodeSDK::Type::MacOSX;
   EXPECT_EQ("macosx", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::iPhoneSimulator;
   EXPECT_EQ("iphonesimulator", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::iPhoneOS;
   EXPECT_EQ("iphoneos", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::AppleTVSimulator;
   EXPECT_EQ("appletvsimulator", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::AppleTVOS;
   EXPECT_EQ("appletvos", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::WatchSimulator;
   EXPECT_EQ("watchsimulator", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::watchOS;
   EXPECT_EQ("watchos", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::Linux;
   EXPECT_EQ("linux", XcodeSDK::GetCanonicalName(info));
-
-  info.type = XcodeSDK::Type::numSDKTypes;
-  EXPECT_EQ("", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::unknown;
   EXPECT_EQ("", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.internal = true;
   info.type = XcodeSDK::Type::MacOSX;
   EXPECT_EQ("macosx.internal", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::iPhoneSimulator;
   EXPECT_EQ("iphonesimulator.internal", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::iPhoneOS;
   EXPECT_EQ("iphoneos.internal", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::AppleTVSimulator;
   EXPECT_EQ("appletvsimulator.internal", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::AppleTVOS;
   EXPECT_EQ("appletvos.internal", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::WatchSimulator;
   EXPECT_EQ("watchsimulator.internal", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::watchOS;
   EXPECT_EQ("watchos.internal", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::MacOSX;
   info.version = llvm::VersionTuple(10, 9);
   EXPECT_EQ("macosx10.9.internal", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 
   info.type = XcodeSDK::Type::iPhoneOS;
   info.version = llvm::VersionTuple(7, 0);
   EXPECT_EQ("iphoneos7.0.internal", XcodeSDK::GetCanonicalName(info));
+  EXPECT_EQ(XcodeSDK(info).Parse(), info);
 }
 
 TEST(XcodeSDKTest, GetSDKTypeForTriple) {
Index: lldb/source/Utility/XcodeSDK.cpp
===
--- lldb/source/Utility/XcodeSDK.cpp
+++ lldb/source/Utility/XcodeSDK.cpp
@@ -18,6 +18,43 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static llvm::StringRef GetName(XcodeSDK::Type type) {
+  switch (type) {
+  case XcodeSDK::MacOSX:
+return "MacOSX";
+  case XcodeSDK::iPhoneSimulator:
+return "iPhoneSimulator";
+  case XcodeSDK::iPhoneOS:
+return "iPhoneOS";
+  case XcodeSDK::AppleTVSimulator:
+return "AppleTVSimulator";
+  case XcodeSDK::AppleTVOS:
+return "AppleTVOS";
+  case XcodeSDK::WatchSimulator:
+return "WatchSimulator";
+  case XcodeSDK::watchOS:
+return "WatchOS";
+  case XcodeSDK::bridgeOS:
+return "bridgeOS";
+  case XcodeSDK::Linux:
+return "Linux";
+  case XcodeSDK::unknown:
+return {};
+  }
+  static_assert(XcodeSDK::Linux == XcodeSDK::numSDKTypes - 1,
+"New SDK type was added, update this list!");
+}
+
+XcodeSDK::XcodeSDK(XcodeSDK::Info info) : m_name(GetName(info.type).str()) {
+  if 

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:2846
+  if (llvm::isa(compiler_type.GetTypeSystem())) {
+if (HasSyntheticValue()) {
+  TargetSP target_sp = GetTargetSP();

mib wrote:
> friss wrote:
> > I am understanding correctly that this condition will only trigger when 
> > there's a synthetic child provider for the type? In other words, if we have 
> > an opaque pointer that we don't know how to display, it will still cause an 
> > error, right?
> Yes, from my understanding, if the opaque pointer doesn't have any synthetic 
> child provider, it won't have a synthetic value and lldb should error saying 
> `error: dereference failed`.
We should add a test to make sure this stays true.  



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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


[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib marked 4 inline comments as done.
mib added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:52
 
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+

friss wrote:
> You don't want to introduce a dependency between Core and a plugin. What you 
> need here might need to be introduced as a new abstract functionality on the 
> TypeSystem base class.
I'll change this before updating the differential with the fixes.



Comment at: lldb/source/Core/ValueObject.cpp:2846
+  if (llvm::isa(compiler_type.GetTypeSystem())) {
+if (HasSyntheticValue()) {
+  TargetSP target_sp = GetTargetSP();

friss wrote:
> I am understanding correctly that this condition will only trigger when 
> there's a synthetic child provider for the type? In other words, if we have 
> an opaque pointer that we don't know how to display, it will still cause an 
> error, right?
Yes, from my understanding, if the opaque pointer doesn't have any synthetic 
child provider, it won't have a synthetic value and lldb should error saying 
`error: dereference failed`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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


[Lldb-commits] [PATCH] D79535: Add a function to detect whether an Xcode SDK supports Swift

2020-05-07 Thread Adrian Prantl via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdec1c94e801f: Add a function to detect whether an Xcode SDK 
supports Swift (authored by aprantl).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D79535?vs=262699=262715#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79535

Files:
  lldb/include/lldb/Utility/XcodeSDK.h
  lldb/source/Utility/XcodeSDK.cpp
  lldb/unittests/Utility/XcodeSDKTest.cpp


Index: lldb/unittests/Utility/XcodeSDKTest.cpp
===
--- lldb/unittests/Utility/XcodeSDKTest.cpp
+++ lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -86,6 +86,17 @@
 }
 #endif
 
+TEST(XcodeSDKTest, SDKSupportsSwift) {
+  EXPECT_TRUE(XcodeSDK("iPhoneSimulator12.0.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("iPhoneSimulator12.0.Internal.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("iPhoneSimulator7.2.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("MacOSX10.10.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("MacOSX10.9.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("Linux.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("MacOSX.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("EverythingElse.sdk").SupportsSwift());
+}
+
 TEST(XcodeSDKTest, GetCanonicalName) {
   XcodeSDK::Info info;
   info.type = XcodeSDK::Type::MacOSX;
Index: lldb/source/Utility/XcodeSDK.cpp
===
--- lldb/source/Utility/XcodeSDK.cpp
+++ lldb/source/Utility/XcodeSDK.cpp
@@ -181,6 +181,27 @@
   return false;
 }
 
+bool XcodeSDK::SupportsSwift() const {
+  XcodeSDK::Info info = Parse();
+  switch (info.type) {
+  case Type::MacOSX:
+return info.version.empty() || info.version >= llvm::VersionTuple(10, 10);
+  case Type::iPhoneOS:
+  case Type::iPhoneSimulator:
+return info.version.empty() || info.version >= llvm::VersionTuple(8);
+  case Type::AppleTVSimulator:
+  case Type::AppleTVOS:
+return info.version.empty() || info.version >= llvm::VersionTuple(9);
+  case Type::WatchSimulator:
+  case Type::watchOS:
+return info.version.empty() || info.version >= llvm::VersionTuple(2);
+  case Type::Linux:
+return true;
+  default:
+return false;
+  }
+}
+
 bool XcodeSDK::SDKSupportsModules(XcodeSDK::Type desired_type,
   const FileSpec _path) {
   ConstString last_path_component = sdk_path.GetLastPathComponent();
Index: lldb/include/lldb/Utility/XcodeSDK.h
===
--- lldb/include/lldb/Utility/XcodeSDK.h
+++ lldb/include/lldb/Utility/XcodeSDK.h
@@ -71,7 +71,10 @@
   llvm::VersionTuple GetVersion() const;
   Type GetType() const;
   llvm::StringRef GetString() const;
+  /// Whether this Xcode SDK supports Swift.
+  bool SupportsSwift() const;
 
+  /// Whether LLDB feels confident importing Clang modules from this SDK.
   static bool SDKSupportsModules(Type type, llvm::VersionTuple version);
   static bool SDKSupportsModules(Type desired_type, const FileSpec _path);
   /// Return the canonical SDK name, such as "macosx" for the macOS SDK.


Index: lldb/unittests/Utility/XcodeSDKTest.cpp
===
--- lldb/unittests/Utility/XcodeSDKTest.cpp
+++ lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -86,6 +86,17 @@
 }
 #endif
 
+TEST(XcodeSDKTest, SDKSupportsSwift) {
+  EXPECT_TRUE(XcodeSDK("iPhoneSimulator12.0.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("iPhoneSimulator12.0.Internal.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("iPhoneSimulator7.2.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("MacOSX10.10.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("MacOSX10.9.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("Linux.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("MacOSX.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("EverythingElse.sdk").SupportsSwift());
+}
+
 TEST(XcodeSDKTest, GetCanonicalName) {
   XcodeSDK::Info info;
   info.type = XcodeSDK::Type::MacOSX;
Index: lldb/source/Utility/XcodeSDK.cpp
===
--- lldb/source/Utility/XcodeSDK.cpp
+++ lldb/source/Utility/XcodeSDK.cpp
@@ -181,6 +181,27 @@
   return false;
 }
 
+bool XcodeSDK::SupportsSwift() const {
+  XcodeSDK::Info info = Parse();
+  switch (info.type) {
+  case Type::MacOSX:
+return info.version.empty() || info.version >= llvm::VersionTuple(10, 10);
+  case Type::iPhoneOS:
+  case Type::iPhoneSimulator:
+return info.version.empty() || info.version >= llvm::VersionTuple(8);
+  case Type::AppleTVSimulator:
+  case Type::AppleTVOS:
+return info.version.empty() || info.version >= llvm::VersionTuple(9);
+  case Type::WatchSimulator:
+  case Type::watchOS:
+return info.version.empty() || info.version >= llvm::VersionTuple(2);
+  case Type::Linux:
+return 

[Lldb-commits] [lldb] e6fbce6 - [lldb/Test] Fix typo in find-and-replace.

2020-05-07 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-05-07T11:54:29-07:00
New Revision: e6fbce675d99982dab2e00eab390359e306d00a8

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

LOG: [lldb/Test] Fix typo in find-and-replace.

Added: 


Modified: 
lldb/test/API/python_api/file_handle/TestFileHandle.py

Removed: 




diff  --git a/lldb/test/API/python_api/file_handle/TestFileHandle.py 
b/lldb/test/API/python_api/file_handle/TestFileHandle.py
index 98e8847467a0..bbcb1124b74a 100644
--- a/lldb/test/API/python_api/file_handle/TestFileHandle.py
+++ b/lldb/test/API/python_api/file_handle/TestFileHandle.py
@@ -120,7 +120,7 @@ def handleCmd(self, cmd, check=True, collect_result=True):
 
 
 @add_test_categories(['pyapi'])
-@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_legacy_file_out_script(self):
 with open(self.out_filename, 'w') as f:
 self.dbg.SetOutputFileHandle(f, False)
@@ -136,7 +136,7 @@ def test_legacy_file_out_script(self):
 
 
 @add_test_categories(['pyapi'])
-@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_legacy_file_out(self):
 with open(self.out_filename, 'w') as f:
 self.dbg.SetOutputFileHandle(f, False)
@@ -145,7 +145,7 @@ def test_legacy_file_out(self):
 self.assertIn('deadbeef', f.read())
 
 @add_test_categories(['pyapi'])
-@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_legacy_file_err_with_get(self):
 with open(self.out_filename, 'w') as f:
 self.dbg.SetErrorFileHandle(f, False)
@@ -160,7 +160,7 @@ def test_legacy_file_err_with_get(self):
 
 
 @add_test_categories(['pyapi'])
-@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_legacy_file_err(self):
 with open(self.out_filename, 'w') as f:
 self.dbg.SetErrorFileHandle(f, False)
@@ -170,7 +170,7 @@ def test_legacy_file_err(self):
 
 
 @add_test_categories(['pyapi'])
-@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_legacy_file_error(self):
 with open(self.out_filename, 'w') as f:
 self.dbg.SetErrorFileHandle(f, False)
@@ -180,7 +180,7 @@ def test_legacy_file_error(self):
 self.assertTrue(re.search(r'error:.*lolwut', errors))
 
 @add_test_categories(['pyapi'])
-@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_sbfile_type_errors(self):
 sbf = lldb.SBFile()
 self.assertRaises(Exception, sbf.Write, None)
@@ -191,7 +191,7 @@ def test_sbfile_type_errors(self):
 
 
 @add_test_categories(['pyapi'])
-@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_sbfile_write_fileno(self):
 with open(self.out_filename, 'w') as f:
 sbf = lldb.SBFile(f.fileno(), "w", False)
@@ -206,7 +206,7 @@ def test_sbfile_write_fileno(self):
 
 
 @add_test_categories(['pyapi'])
-@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_sbfile_write(self):
 with open(self.out_filename, 'w') as f:
 sbf = lldb.SBFile(f)
@@ -220,7 +220,7 @@ def test_sbfile_write(self):
 
 
 @add_test_categories(['pyapi'])
-@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_sbfile_read_fileno(self):
 with open(self.out_filename, 'w') as f:
 f.write('FOO')
@@ -234,7 +234,7 @@ def test_sbfile_read_fileno(self):
 
 
 @add_test_categories(['pyapi'])
-@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_sbfile_read(self):
 with open(self.out_filename, 'w') as f:
 f.write('foo')
@@ -250,7 +250,7 @@ def test_sbfile_read(self):
 
 
 @add_test_categories(['pyapi'])
-@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+@skipIfReproducer # 

[Lldb-commits] [lldb] dec1c94 - Add a function to detect whether an Xcode SDK supports Swift

2020-05-07 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2020-05-07T11:29:31-07:00
New Revision: dec1c94e801f6fe1bae01c4679aca67abe0cb8a6

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

LOG: Add a function to detect whether an Xcode SDK supports Swift

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

Added: 


Modified: 
lldb/include/lldb/Utility/XcodeSDK.h
lldb/source/Utility/XcodeSDK.cpp
lldb/unittests/Utility/XcodeSDKTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/XcodeSDK.h 
b/lldb/include/lldb/Utility/XcodeSDK.h
index e5a0e0351d83..94a97281b71d 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -71,7 +71,10 @@ class XcodeSDK {
   llvm::VersionTuple GetVersion() const;
   Type GetType() const;
   llvm::StringRef GetString() const;
+  /// Whether this Xcode SDK supports Swift.
+  bool SupportsSwift() const;
 
+  /// Whether LLDB feels confident importing Clang modules from this SDK.
   static bool SDKSupportsModules(Type type, llvm::VersionTuple version);
   static bool SDKSupportsModules(Type desired_type, const FileSpec _path);
   /// Return the canonical SDK name, such as "macosx" for the macOS SDK.

diff  --git a/lldb/source/Utility/XcodeSDK.cpp 
b/lldb/source/Utility/XcodeSDK.cpp
index c6f2dda0b2b8..0d9f98fe52ec 100644
--- a/lldb/source/Utility/XcodeSDK.cpp
+++ b/lldb/source/Utility/XcodeSDK.cpp
@@ -181,6 +181,27 @@ bool XcodeSDK::SDKSupportsModules(XcodeSDK::Type sdk_type,
   return false;
 }
 
+bool XcodeSDK::SupportsSwift() const {
+  XcodeSDK::Info info = Parse();
+  switch (info.type) {
+  case Type::MacOSX:
+return info.version.empty() || info.version >= llvm::VersionTuple(10, 10);
+  case Type::iPhoneOS:
+  case Type::iPhoneSimulator:
+return info.version.empty() || info.version >= llvm::VersionTuple(8);
+  case Type::AppleTVSimulator:
+  case Type::AppleTVOS:
+return info.version.empty() || info.version >= llvm::VersionTuple(9);
+  case Type::WatchSimulator:
+  case Type::watchOS:
+return info.version.empty() || info.version >= llvm::VersionTuple(2);
+  case Type::Linux:
+return true;
+  default:
+return false;
+  }
+}
+
 bool XcodeSDK::SDKSupportsModules(XcodeSDK::Type desired_type,
   const FileSpec _path) {
   ConstString last_path_component = sdk_path.GetLastPathComponent();

diff  --git a/lldb/unittests/Utility/XcodeSDKTest.cpp 
b/lldb/unittests/Utility/XcodeSDKTest.cpp
index e89eac2ef2f7..d9917194324c 100644
--- a/lldb/unittests/Utility/XcodeSDKTest.cpp
+++ b/lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -86,6 +86,17 @@ TEST(XcodeSDKTest, SDKSupportsModules) {
 }
 #endif
 
+TEST(XcodeSDKTest, SDKSupportsSwift) {
+  EXPECT_TRUE(XcodeSDK("iPhoneSimulator12.0.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("iPhoneSimulator12.0.Internal.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("iPhoneSimulator7.2.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("MacOSX10.10.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("MacOSX10.9.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("Linux.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("MacOSX.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("EverythingElse.sdk").SupportsSwift());
+}
+
 TEST(XcodeSDKTest, GetCanonicalName) {
   XcodeSDK::Info info;
   info.type = XcodeSDK::Type::MacOSX;



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


[Lldb-commits] [lldb] 8c0ff17 - [lldb/Test] Add @skipIfReproducer to tests using lldb::FileSP.

2020-05-07 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-05-07T11:17:00-07:00
New Revision: 8c0ff17c3bb44edf455a2964e6035f28be68c9f3

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

LOG: [lldb/Test] Add @skipIfReproducer to tests using lldb::FileSP.

lldb::FileSP is a typedef for std::shared_ptr and
the reproducers cannot instrument a lldb_private constructor.

Added: 


Modified: 
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
lldb/test/API/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
lldb/test/API/python_api/file_handle/TestFileHandle.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py 
b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
index f209261f0500..3e1abc3353c3 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -29,6 +29,7 @@ def process_from_yaml(self, yaml_file):
 self.process = self.target.LoadCore(minidump_path)
 return self.process
 
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def check_state(self):
 with open(os.devnull) as devnul:
 # sanitize test output

diff  --git 
a/lldb/test/API/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py 
b/lldb/test/API/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
index e4d085de7c2b..fd5c9ec59ea9 100644
--- a/lldb/test/API/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
+++ b/lldb/test/API/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
@@ -20,6 +20,7 @@ class Disassemble_VST1_64(TestBase):
 @add_test_categories(['pyapi'])
 @no_debug_info_test
 @skipIfLLVMTargetMissing("ARM")
+@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_disassemble_invalid_vst_1_64_raw_data(self):
 """Test disassembling invalid vst1.64 raw bytes with the API."""
 # Create a target from the debugger.

diff  --git a/lldb/test/API/python_api/file_handle/TestFileHandle.py 
b/lldb/test/API/python_api/file_handle/TestFileHandle.py
index 550aad2ad8a1..98e8847467a0 100644
--- a/lldb/test/API/python_api/file_handle/TestFileHandle.py
+++ b/lldb/test/API/python_api/file_handle/TestFileHandle.py
@@ -120,6 +120,7 @@ def handleCmd(self, cmd, check=True, collect_result=True):
 
 
 @add_test_categories(['pyapi'])
+@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_legacy_file_out_script(self):
 with open(self.out_filename, 'w') as f:
 self.dbg.SetOutputFileHandle(f, False)
@@ -135,6 +136,7 @@ def test_legacy_file_out_script(self):
 
 
 @add_test_categories(['pyapi'])
+@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_legacy_file_out(self):
 with open(self.out_filename, 'w') as f:
 self.dbg.SetOutputFileHandle(f, False)
@@ -143,6 +145,7 @@ def test_legacy_file_out(self):
 self.assertIn('deadbeef', f.read())
 
 @add_test_categories(['pyapi'])
+@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_legacy_file_err_with_get(self):
 with open(self.out_filename, 'w') as f:
 self.dbg.SetErrorFileHandle(f, False)
@@ -157,6 +160,7 @@ def test_legacy_file_err_with_get(self):
 
 
 @add_test_categories(['pyapi'])
+@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_legacy_file_err(self):
 with open(self.out_filename, 'w') as f:
 self.dbg.SetErrorFileHandle(f, False)
@@ -166,6 +170,7 @@ def test_legacy_file_err(self):
 
 
 @add_test_categories(['pyapi'])
+@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_legacy_file_error(self):
 with open(self.out_filename, 'w') as f:
 self.dbg.SetErrorFileHandle(f, False)
@@ -175,6 +180,7 @@ def test_legacy_file_error(self):
 self.assertTrue(re.search(r'error:.*lolwut', errors))
 
 @add_test_categories(['pyapi'])
+@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_sbfile_type_errors(self):
 sbf = lldb.SBFile()
 self.assertRaises(Exception, sbf.Write, None)
@@ -185,6 +191,7 @@ def test_sbfile_type_errors(self):
 
 
 @add_test_categories(['pyapi'])
+@@skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
 def test_sbfile_write_fileno(self):
 with open(self.out_filename, 'w') as f:
 sbf = lldb.SBFile(f.fileno(), "w", False)
@@ -199,6 +206,7 @@ def test_sbfile_write_fileno(self):
 
 
 

[Lldb-commits] [PATCH] D79538: Add an XcodeSDK::GetSDKTypeForTriple function

2020-05-07 Thread Adrian Prantl via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6e95d51ecfab: Add an XcodeSDK::GetSDKTypeForTriple function 
(authored by aprantl).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D79538?vs=262519=262706#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79538

Files:
  lldb/include/lldb/Utility/XcodeSDK.h
  lldb/source/Utility/XcodeSDK.cpp
  lldb/unittests/Utility/XcodeSDKTest.cpp

Index: lldb/unittests/Utility/XcodeSDKTest.cpp
===
--- lldb/unittests/Utility/XcodeSDKTest.cpp
+++ lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Utility/XcodeSDK.h"
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Triple.h"
 
 #include 
 
@@ -147,3 +148,33 @@
   info.version = llvm::VersionTuple(7, 0);
   EXPECT_EQ("iphoneos7.0.internal", XcodeSDK::GetCanonicalName(info));
 }
+
+TEST(XcodeSDKTest, GetSDKTypeForTriple) {
+  EXPECT_EQ(
+  XcodeSDK::GetSDKTypeForTriple(llvm::Triple("x86_64-apple-macosx10.14")),
+  XcodeSDK::Type::MacOSX);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(llvm::Triple("x86_64-apple-darwin")),
+XcodeSDK::Type::MacOSX);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(
+llvm::Triple("x86_64-apple-ios13.4-simulator")),
+XcodeSDK::Type::iPhoneSimulator);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(llvm::Triple("arm64-apple-ios13.4")),
+XcodeSDK::Type::iPhoneOS);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(
+llvm::Triple("x86_64-apple-ios13.4-macabi")),
+XcodeSDK::Type::MacOSX);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(
+llvm::Triple("x86_64-apple-tvos-simulator")),
+XcodeSDK::Type::AppleTVSimulator);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(llvm::Triple("arm64-apple-tvos")),
+XcodeSDK::Type::AppleTVOS);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(
+llvm::Triple("x86_64-apple-watchos-simulator")),
+XcodeSDK::Type::WatchSimulator);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(llvm::Triple("arm64-apple-watchos")),
+XcodeSDK::Type::watchOS);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(llvm::Triple("x86_64-unknown-linux")),
+XcodeSDK::Type::Linux);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(llvm::Triple("i386-unknown-netbsd")),
+XcodeSDK::Type::unknown);
+}
Index: lldb/source/Utility/XcodeSDK.cpp
===
--- lldb/source/Utility/XcodeSDK.cpp
+++ lldb/source/Utility/XcodeSDK.cpp
@@ -6,10 +6,13 @@
 //
 //===--===//
 
-#include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/XcodeSDK.h"
+#include "lldb/Utility/FileSpec.h"
 
 #include "lldb/lldb-types.h"
+
+#include "llvm/ADT/Triple.h"
+
 #include 
 
 using namespace lldb;
@@ -190,3 +193,33 @@
 return false;
   return SDKSupportsModules(sdk.GetType(), sdk.GetVersion());
 }
+
+XcodeSDK::Type XcodeSDK::GetSDKTypeForTriple(const llvm::Triple ) {
+  using namespace llvm;
+  switch (triple.getOS()) {
+  case Triple::MacOSX:
+  case Triple::Darwin:
+return XcodeSDK::MacOSX;
+  case Triple::IOS:
+switch (triple.getEnvironment()) {
+case Triple::MacABI:
+  return XcodeSDK::MacOSX;
+case Triple::Simulator:
+  return XcodeSDK::iPhoneSimulator;
+default:
+  return XcodeSDK::iPhoneOS;
+}
+  case Triple::TvOS:
+if (triple.getEnvironment() == Triple::Simulator)
+  return XcodeSDK::AppleTVSimulator;
+return XcodeSDK::AppleTVOS;
+  case Triple::WatchOS:
+if (triple.getEnvironment() == Triple::Simulator)
+  return XcodeSDK::WatchSimulator;
+return XcodeSDK::watchOS;
+  case Triple::Linux:
+return XcodeSDK::Linux;
+  default:
+return XcodeSDK::unknown;
+  }
+}
Index: lldb/include/lldb/Utility/XcodeSDK.h
===
--- lldb/include/lldb/Utility/XcodeSDK.h
+++ lldb/include/lldb/Utility/XcodeSDK.h
@@ -14,6 +14,10 @@
 #include "llvm/Support/VersionTuple.h"
 #include 
 
+namespace llvm {
+class Triple;
+}
+
 namespace lldb_private {
 
 /// An abstraction for Xcode-style SDKs that works like \ref ArchSpec.
@@ -72,6 +76,8 @@
   static bool SDKSupportsModules(Type desired_type, const FileSpec _path);
   /// Return the canonical SDK name, such as "macosx" for the macOS SDK.
   static std::string GetCanonicalName(Info info);
+  /// Return the best-matching SDK type for a specific triple.
+  static XcodeSDK::Type GetSDKTypeForTriple(const llvm::Triple );
 };
 
 } // namespace lldb_private
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

The way the ValueObject code works w.r.t. Synthetic child providers is that you 
first look up and make a ValueObject for the actual value the user requested, 
(for instance a ValueObjectVariable or a ValueObjectChild or a 
ValueObjectConstResult for expressions, etc.), then you hand that to the 
printer.  The printer will look at whether the current settings prefer 
Synthetic Values, and if they do they ask the actual value if it has any 
Synthetic children.  So when you do something like:

(lldb) frame var *opaque_ptr

we first have to make a ValueObject for *opaque_ptr (in this case the 
dereference is a ValueObjectChild of the ValueObjectVariable representing the 
variable `opaque_ptr` and return that to the printer.

But you can't currently make a ValueObject that doesn't have a type, and the 
problem here is that we don't have a type for * of the opaque pointer.  Making 
an empty struct definition and returning a ValueObject with that struct 
definition is one fairly straightforward way of doing that.  And we already 
inject custom types in other places to manage synthetic children (look for 
`__lldb_autogen_nspair` for instance).  So this seemed the most straightforward 
choice.

Note, you could also make a new ValueObject subclass for these purposes: 
ValueObjectOpaqueDereference to forward the Synthetic Children.  The 
ValueObject base class doesn't require a type so you could make such a thing. 
But it would somehow fake handing out the type so you'd have to be careful how 
you did that.  Then its job would be to hand out the synthetic children.  If 
people really hate making the fake structure we could go this way instead, but 
it seems better to use an extant facility if we can, and "fake handing out the 
type" seems to me at least as dangerous as injecting synthesized types into the 
TypeSystem.  The latter we already do, the former we don't.

I'm not happy with the notion of just hard-coding this to CF types or making it 
about bridged types at all.  It is not uncommon to have a client library that 
vends opaque pointers, but you've figured out what some of the fields are.  One 
solution to debugging this scenario is to introduce another shadow type, either 
in lldb or actually in your code, and cast types to that when printing.  But 
that means you can't use "frame var" since it doesn't support casting, and you 
have to remember to cast every time in the expression parser which is annoying. 
 if you made a synthetic child provider, and if what Ismail is trying to get 
working here works generally, then once you've made the provider, you can just 
print * of the pointer, and it will work.  So I think this should be a general 
facility for opaque pointer types with synthetic child providers.  So I'd much 
prefer to keep this a general facility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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


[Lldb-commits] [PATCH] D79535: Add a function to detect whether an Xcode SDK supports Swift

2020-05-07 Thread Vedant Kumar via Phabricator via lldb-commits
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

Nice test!


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

https://reviews.llvm.org/D79535



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


[Lldb-commits] [PATCH] D79538: Add an XcodeSDK::GetSDKTypeForTriple function

2020-05-07 Thread Vedant Kumar via Phabricator via lldb-commits
vsk accepted this revision.
vsk added a comment.

lgtm as well


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

https://reviews.llvm.org/D79538



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


[Lldb-commits] [PATCH] D79535: Add a function to detect whether an Xcode SDK supports Swift

2020-05-07 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl updated this revision to Diff 262699.
aprantl marked an inline comment as done.

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

https://reviews.llvm.org/D79535

Files:
  lldb/include/lldb/Utility/XcodeSDK.h
  lldb/source/Utility/XcodeSDK.cpp
  lldb/unittests/Utility/XcodeSDKTest.cpp


Index: lldb/unittests/Utility/XcodeSDKTest.cpp
===
--- lldb/unittests/Utility/XcodeSDKTest.cpp
+++ lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -85,6 +85,17 @@
 }
 #endif
 
+TEST(XcodeSDKTest, SDKSupportsSwift) {
+  EXPECT_TRUE(XcodeSDK("iPhoneSimulator12.0.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("iPhoneSimulator12.0.Internal.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("iPhoneSimulator7.2.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("MacOSX10.10.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("MacOSX10.9.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("Linux.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("MacOSX.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("EverythingElse.sdk").SupportsSwift());
+}
+
 TEST(XcodeSDKTest, GetCanonicalName) {
   XcodeSDK::Info info;
   info.type = XcodeSDK::Type::MacOSX;
Index: lldb/source/Utility/XcodeSDK.cpp
===
--- lldb/source/Utility/XcodeSDK.cpp
+++ lldb/source/Utility/XcodeSDK.cpp
@@ -178,6 +178,27 @@
   return false;
 }
 
+bool XcodeSDK::SupportsSwift() const {
+  XcodeSDK::Info info = Parse();
+  switch (info.type) {
+  case Type::MacOSX:
+return info.version.empty() || info.version >= llvm::VersionTuple(10, 10);
+  case Type::iPhoneOS:
+  case Type::iPhoneSimulator:
+return info.version.empty() || info.version >= llvm::VersionTuple(8);
+  case XcodeSDK::Type::AppleTVSimulator:
+  case XcodeSDK::Type::AppleTVOS:
+return info.version.empty() || info.version >= llvm::VersionTuple(9);
+  case XcodeSDK::Type::WatchSimulator:
+  case XcodeSDK::Type::watchOS:
+return info.version.empty() || info.version >= llvm::VersionTuple(2);
+  case XcodeSDK::Type::Linux:
+return true;
+  default:
+return false;
+  }
+}
+
 bool XcodeSDK::SDKSupportsModules(XcodeSDK::Type desired_type,
   const FileSpec _path) {
   ConstString last_path_component = sdk_path.GetLastPathComponent();
Index: lldb/include/lldb/Utility/XcodeSDK.h
===
--- lldb/include/lldb/Utility/XcodeSDK.h
+++ lldb/include/lldb/Utility/XcodeSDK.h
@@ -67,7 +67,10 @@
   llvm::VersionTuple GetVersion() const;
   Type GetType() const;
   llvm::StringRef GetString() const;
+  /// Whether this Xcode SDK supports Swift.
+  bool SupportsSwift() const;
 
+  /// Whether LLDB feels confident importing Clang modules from this SDK.
   static bool SDKSupportsModules(Type type, llvm::VersionTuple version);
   static bool SDKSupportsModules(Type desired_type, const FileSpec _path);
   /// Return the canonical SDK name, such as "macosx" for the macOS SDK.


Index: lldb/unittests/Utility/XcodeSDKTest.cpp
===
--- lldb/unittests/Utility/XcodeSDKTest.cpp
+++ lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -85,6 +85,17 @@
 }
 #endif
 
+TEST(XcodeSDKTest, SDKSupportsSwift) {
+  EXPECT_TRUE(XcodeSDK("iPhoneSimulator12.0.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("iPhoneSimulator12.0.Internal.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("iPhoneSimulator7.2.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("MacOSX10.10.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("MacOSX10.9.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("Linux.sdk").SupportsSwift());
+  EXPECT_TRUE(XcodeSDK("MacOSX.sdk").SupportsSwift());
+  EXPECT_FALSE(XcodeSDK("EverythingElse.sdk").SupportsSwift());
+}
+
 TEST(XcodeSDKTest, GetCanonicalName) {
   XcodeSDK::Info info;
   info.type = XcodeSDK::Type::MacOSX;
Index: lldb/source/Utility/XcodeSDK.cpp
===
--- lldb/source/Utility/XcodeSDK.cpp
+++ lldb/source/Utility/XcodeSDK.cpp
@@ -178,6 +178,27 @@
   return false;
 }
 
+bool XcodeSDK::SupportsSwift() const {
+  XcodeSDK::Info info = Parse();
+  switch (info.type) {
+  case Type::MacOSX:
+return info.version.empty() || info.version >= llvm::VersionTuple(10, 10);
+  case Type::iPhoneOS:
+  case Type::iPhoneSimulator:
+return info.version.empty() || info.version >= llvm::VersionTuple(8);
+  case XcodeSDK::Type::AppleTVSimulator:
+  case XcodeSDK::Type::AppleTVOS:
+return info.version.empty() || info.version >= llvm::VersionTuple(9);
+  case XcodeSDK::Type::WatchSimulator:
+  case XcodeSDK::Type::watchOS:
+return info.version.empty() || info.version >= llvm::VersionTuple(2);
+  case XcodeSDK::Type::Linux:
+return true;
+  default:
+return false;
+  }
+}
+
 bool XcodeSDK::SDKSupportsModules(XcodeSDK::Type desired_type,
   const 

[Lldb-commits] [PATCH] D79535: Add a function to detect whether an Xcode SDK supports Swift

2020-05-07 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: lldb/include/lldb/Utility/XcodeSDK.h:70
   llvm::StringRef GetString() const;
+  bool SupportsModules() const;
+  /// Whether this Xcode SDK supports Swift.

JDevlieghere wrote:
> Not implemented?
I had another commit that changed SDKSupportsModules into a member function, 
but then realized that this wouldn't gel with the way the function is used by 
PlatformDarwin.

Removed.


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

https://reviews.llvm.org/D79535



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


[Lldb-commits] [lldb] 6e95d51 - Add an XcodeSDK::GetSDKTypeForTriple function

2020-05-07 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2020-05-07T11:12:42-07:00
New Revision: 6e95d51ecfab0801b75448b20bd3447645a1142a

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

LOG: Add an XcodeSDK::GetSDKTypeForTriple function

This is something used in swift-lldb, but of general usefulness.

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



Added: 


Modified: 
lldb/include/lldb/Utility/XcodeSDK.h
lldb/source/Utility/XcodeSDK.cpp
lldb/unittests/Utility/XcodeSDKTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/XcodeSDK.h 
b/lldb/include/lldb/Utility/XcodeSDK.h
index 24ab5b1fdf7a..e5a0e0351d83 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -14,6 +14,10 @@
 #include "llvm/Support/VersionTuple.h"
 #include 
 
+namespace llvm {
+class Triple;
+}
+
 namespace lldb_private {
 
 /// An abstraction for Xcode-style SDKs that works like \ref ArchSpec.
@@ -72,6 +76,8 @@ class XcodeSDK {
   static bool SDKSupportsModules(Type desired_type, const FileSpec _path);
   /// Return the canonical SDK name, such as "macosx" for the macOS SDK.
   static std::string GetCanonicalName(Info info);
+  /// Return the best-matching SDK type for a specific triple.
+  static XcodeSDK::Type GetSDKTypeForTriple(const llvm::Triple );
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Utility/XcodeSDK.cpp 
b/lldb/source/Utility/XcodeSDK.cpp
index fc1fc32b059c..c6f2dda0b2b8 100644
--- a/lldb/source/Utility/XcodeSDK.cpp
+++ b/lldb/source/Utility/XcodeSDK.cpp
@@ -6,10 +6,13 @@
 //
 
//===--===//
 
-#include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/XcodeSDK.h"
+#include "lldb/Utility/FileSpec.h"
 
 #include "lldb/lldb-types.h"
+
+#include "llvm/ADT/Triple.h"
+
 #include 
 
 using namespace lldb;
@@ -190,3 +193,33 @@ bool XcodeSDK::SDKSupportsModules(XcodeSDK::Type 
desired_type,
 return false;
   return SDKSupportsModules(sdk.GetType(), sdk.GetVersion());
 }
+
+XcodeSDK::Type XcodeSDK::GetSDKTypeForTriple(const llvm::Triple ) {
+  using namespace llvm;
+  switch (triple.getOS()) {
+  case Triple::MacOSX:
+  case Triple::Darwin:
+return XcodeSDK::MacOSX;
+  case Triple::IOS:
+switch (triple.getEnvironment()) {
+case Triple::MacABI:
+  return XcodeSDK::MacOSX;
+case Triple::Simulator:
+  return XcodeSDK::iPhoneSimulator;
+default:
+  return XcodeSDK::iPhoneOS;
+}
+  case Triple::TvOS:
+if (triple.getEnvironment() == Triple::Simulator)
+  return XcodeSDK::AppleTVSimulator;
+return XcodeSDK::AppleTVOS;
+  case Triple::WatchOS:
+if (triple.getEnvironment() == Triple::Simulator)
+  return XcodeSDK::WatchSimulator;
+return XcodeSDK::watchOS;
+  case Triple::Linux:
+return XcodeSDK::Linux;
+  default:
+return XcodeSDK::unknown;
+  }
+}

diff  --git a/lldb/unittests/Utility/XcodeSDKTest.cpp 
b/lldb/unittests/Utility/XcodeSDKTest.cpp
index 0cc353aa1ff7..e89eac2ef2f7 100644
--- a/lldb/unittests/Utility/XcodeSDKTest.cpp
+++ b/lldb/unittests/Utility/XcodeSDKTest.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Utility/XcodeSDK.h"
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Triple.h"
 
 #include 
 
@@ -147,3 +148,33 @@ TEST(XcodeSDKTest, GetCanonicalName) {
   info.version = llvm::VersionTuple(7, 0);
   EXPECT_EQ("iphoneos7.0.internal", XcodeSDK::GetCanonicalName(info));
 }
+
+TEST(XcodeSDKTest, GetSDKTypeForTriple) {
+  EXPECT_EQ(
+  XcodeSDK::GetSDKTypeForTriple(llvm::Triple("x86_64-apple-macosx10.14")),
+  XcodeSDK::Type::MacOSX);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(llvm::Triple("x86_64-apple-darwin")),
+XcodeSDK::Type::MacOSX);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(
+llvm::Triple("x86_64-apple-ios13.4-simulator")),
+XcodeSDK::Type::iPhoneSimulator);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(llvm::Triple("arm64-apple-ios13.4")),
+XcodeSDK::Type::iPhoneOS);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(
+llvm::Triple("x86_64-apple-ios13.4-macabi")),
+XcodeSDK::Type::MacOSX);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(
+llvm::Triple("x86_64-apple-tvos-simulator")),
+XcodeSDK::Type::AppleTVSimulator);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(llvm::Triple("arm64-apple-tvos")),
+XcodeSDK::Type::AppleTVOS);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(
+llvm::Triple("x86_64-apple-watchos-simulator")),
+XcodeSDK::Type::WatchSimulator);
+  EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(llvm::Triple("arm64-apple-watchos")),
+XcodeSDK::Type::watchOS);
+  
EXPECT_EQ(XcodeSDK::GetSDKTypeForTriple(llvm::Triple("x86_64-unknown-linux")),
+

[Lldb-commits] [PATCH] D79533: Reuse existing functionality in XcodeSDK::SDKSupportsModules (NFC)

2020-05-07 Thread Adrian Prantl via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4356aa20bcea: Reuse existing functionality in 
XcodeSDK::SDKSupportsModules (NFC) (authored by aprantl).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79533

Files:
  lldb/source/Utility/XcodeSDK.cpp


Index: lldb/source/Utility/XcodeSDK.cpp
===
--- lldb/source/Utility/XcodeSDK.cpp
+++ lldb/source/Utility/XcodeSDK.cpp
@@ -182,25 +182,11 @@
   const FileSpec _path) {
   ConstString last_path_component = sdk_path.GetLastPathComponent();
 
-  if (last_path_component) {
-const llvm::StringRef sdk_name = last_path_component.GetStringRef();
-
-const std::string sdk_name_lower = sdk_name.lower();
-Info info;
-info.type = desired_type;
-const std::string sdk_string = GetCanonicalName(info);
-if (!llvm::StringRef(sdk_name_lower).startswith(sdk_string))
-  return false;
-
-auto version_part = sdk_name.drop_front(sdk_string.size());
-version_part.consume_back(".sdk");
-version_part.consume_back(".Internal");
-
-llvm::VersionTuple version;
-if (version.tryParse(version_part))
-  return false;
-return SDKSupportsModules(desired_type, version);
-  }
+  if (!last_path_component)
+return false;
 
-  return false;
+  XcodeSDK sdk(last_path_component.GetStringRef().str());
+  if (sdk.GetType() != desired_type)
+return false;
+  return SDKSupportsModules(sdk.GetType(), sdk.GetVersion());
 }


Index: lldb/source/Utility/XcodeSDK.cpp
===
--- lldb/source/Utility/XcodeSDK.cpp
+++ lldb/source/Utility/XcodeSDK.cpp
@@ -182,25 +182,11 @@
   const FileSpec _path) {
   ConstString last_path_component = sdk_path.GetLastPathComponent();
 
-  if (last_path_component) {
-const llvm::StringRef sdk_name = last_path_component.GetStringRef();
-
-const std::string sdk_name_lower = sdk_name.lower();
-Info info;
-info.type = desired_type;
-const std::string sdk_string = GetCanonicalName(info);
-if (!llvm::StringRef(sdk_name_lower).startswith(sdk_string))
-  return false;
-
-auto version_part = sdk_name.drop_front(sdk_string.size());
-version_part.consume_back(".sdk");
-version_part.consume_back(".Internal");
-
-llvm::VersionTuple version;
-if (version.tryParse(version_part))
-  return false;
-return SDKSupportsModules(desired_type, version);
-  }
+  if (!last_path_component)
+return false;
 
-  return false;
+  XcodeSDK sdk(last_path_component.GetStringRef().str());
+  if (sdk.GetType() != desired_type)
+return false;
+  return SDKSupportsModules(sdk.GetType(), sdk.GetVersion());
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 4356aa2 - Reuse existing functionality in XcodeSDK::SDKSupportsModules (NFC)

2020-05-07 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2020-05-07T10:46:51-07:00
New Revision: 4356aa20bcea05b0f89c9977e41076552985f8c2

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

LOG: Reuse existing functionality in XcodeSDK::SDKSupportsModules (NFC)

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

Added: 


Modified: 
lldb/source/Utility/XcodeSDK.cpp

Removed: 




diff  --git a/lldb/source/Utility/XcodeSDK.cpp 
b/lldb/source/Utility/XcodeSDK.cpp
index 197eef182f69..fc1fc32b059c 100644
--- a/lldb/source/Utility/XcodeSDK.cpp
+++ b/lldb/source/Utility/XcodeSDK.cpp
@@ -182,25 +182,11 @@ bool XcodeSDK::SDKSupportsModules(XcodeSDK::Type 
desired_type,
   const FileSpec _path) {
   ConstString last_path_component = sdk_path.GetLastPathComponent();
 
-  if (last_path_component) {
-const llvm::StringRef sdk_name = last_path_component.GetStringRef();
-
-const std::string sdk_name_lower = sdk_name.lower();
-Info info;
-info.type = desired_type;
-const std::string sdk_string = GetCanonicalName(info);
-if (!llvm::StringRef(sdk_name_lower).startswith(sdk_string))
-  return false;
-
-auto version_part = sdk_name.drop_front(sdk_string.size());
-version_part.consume_back(".sdk");
-version_part.consume_back(".Internal");
-
-llvm::VersionTuple version;
-if (version.tryParse(version_part))
-  return false;
-return SDKSupportsModules(desired_type, version);
-  }
+  if (!last_path_component)
+return false;
 
-  return false;
+  XcodeSDK sdk(last_path_component.GetStringRef().str());
+  if (sdk.GetType() != desired_type)
+return false;
+  return SDKSupportsModules(sdk.GetType(), sdk.GetVersion());
 }



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


[Lldb-commits] [PATCH] D79563: [lldb/test] Make "inline" tests handle multiple statements at the same location

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

Nice


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79563



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


[Lldb-commits] [PATCH] D79308: [lldb-server] Reset stop reason of all threads when resuming

2020-05-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision.
jingham added a comment.

LGTM too!


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

https://reviews.llvm.org/D79308



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


[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

2020-05-07 Thread Vedant Kumar via Phabricator via lldb-commits
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

LGTM, thanks. It'd be nice to have a skipUnlessEntryValuesSupportedArch 
decorator, but I don't think that has to be folded into this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491



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


[Lldb-commits] [PATCH] D79563: [lldb/test] Make "inline" tests handle multiple statements at the same location

2020-05-07 Thread Vedant Kumar via Phabricator via lldb-commits
vsk added a comment.

Thanks! LGTM, modulo any comments Raphael may have about the test change.




Comment at: lldb/packages/Python/lldbsuite/test/lldbinline.py:155
 process, lldb.eStopReasonBreakpoint)
-breakpoint_id = thread.GetStopReasonDataAtIndex(0)
-parser.handle_breakpoint(self, breakpoint_id)
+for id in self._get_breakpoint_ids(thread):
+parser.handle_breakpoint(self, id)

nit: I think this leaves the 'id' builtin defined as the last breakpoint id 
after the loop is done, maybe name it something else ('breakpoint_id'?).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79563



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


[Lldb-commits] [PATCH] D79535: Add a function to detect whether an Xcode SDK supports Swift

2020-05-07 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/XcodeSDK.h:70
   llvm::StringRef GetString() const;
+  bool SupportsModules() const;
+  /// Whether this Xcode SDK supports Swift.

Not implemented?


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

https://reviews.llvm.org/D79535



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


[Lldb-commits] [PATCH] D79586: Do not list adb devices when a device id is given

2020-05-07 Thread Emre Kultursay via Phabricator via lldb-commits
emrekultursay created this revision.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.

On Android, this method gets called twice: first when establishing
a host-server connection, then when attaching to a process id.

Each call takes several seconds to finish (especially slower on Windows)
and eliminating the call for the typical case improves latency significantly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79586

Files:
  lldb/source/Plugins/Platform/Android/AdbClient.cpp
  lldb/unittests/Platform/Android/AdbClientTest.cpp
  lldb/unittests/Platform/Android/CMakeLists.txt
  lldb/unittests/Platform/CMakeLists.txt

Index: lldb/unittests/Platform/CMakeLists.txt
===
--- lldb/unittests/Platform/CMakeLists.txt
+++ lldb/unittests/Platform/CMakeLists.txt
@@ -6,3 +6,5 @@
   LINK_COMPONENTS
 Support
   )
+
+add_subdirectory(Android)
Index: lldb/unittests/Platform/Android/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Platform/Android/CMakeLists.txt
@@ -0,0 +1,8 @@
+include_directories(${LLDB_SOURCE_DIR}/source/Plugins/Platform/Android)
+
+add_lldb_unittest(AdbClientTest
+  AdbClientTest.cpp
+
+  LINK_LIBS
+lldbPluginPlatformAndroid
+  )
Index: lldb/unittests/Platform/Android/AdbClientTest.cpp
===
--- /dev/null
+++ lldb/unittests/Platform/Android/AdbClientTest.cpp
@@ -0,0 +1,45 @@
+//===-- AdbClientTest.cpp -===//
+//
+// 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/Platform/Android/AdbClient.h"
+
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace lldb_private {
+namespace platform_android {
+
+class AdbClientTest : public ::testing::Test {
+public:
+  void SetUp() override { putenv("ANDROID_SERIAL="); }
+
+  void TearDown() override { putenv("ANDROID_SERIAL="); }
+};
+
+TEST(AdbClientTest, CreateByDeviceId) {
+  AdbClient adb;
+  Status error = AdbClient::CreateByDeviceID("device1", adb);
+  EXPECT_TRUE(error.Success());
+  EXPECT_EQ("device1", adb.GetDeviceID());
+}
+
+TEST(AdbClientTest, CreateByDeviceId_ByEnvVar) {
+  putenv("ANDROID_SERIAL=device2");
+
+  AdbClient adb;
+  Status error = AdbClient::CreateByDeviceID("", adb);
+  EXPECT_TRUE(error.Success());
+  EXPECT_EQ("device2", adb.GetDeviceID());
+}
+
+} // end namespace platform_android
+} // end namespace lldb_private
\ No newline at end of file
Index: lldb/source/Plugins/Platform/Android/AdbClient.cpp
===
--- lldb/source/Plugins/Platform/Android/AdbClient.cpp
+++ lldb/source/Plugins/Platform/Android/AdbClient.cpp
@@ -94,11 +94,7 @@
 
 Status AdbClient::CreateByDeviceID(const std::string _id,
AdbClient ) {
-  DeviceIDList connect_devices;
-  auto error = adb.GetDevices(connect_devices);
-  if (error.Fail())
-return error;
-
+  Status error;
   std::string android_serial;
   if (!device_id.empty())
 android_serial = device_id;
@@ -106,18 +102,18 @@
 android_serial = env_serial;
 
   if (android_serial.empty()) {
-if (connect_devices.size() != 1)
+DeviceIDList connected_devices;
+error = adb.GetDevices(connected_devices);
+if (error.Fail())
+  return error;
+
+if (connected_devices.size() != 1)
   return Status("Expected a single connected device, got instead %zu - try "
 "setting 'ANDROID_SERIAL'",
-connect_devices.size());
-adb.SetDeviceID(connect_devices.front());
+connected_devices.size());
+adb.SetDeviceID(connected_devices.front());
   } else {
-auto find_it = std::find(connect_devices.begin(), connect_devices.end(),
- android_serial);
-if (find_it == connect_devices.end())
-  return Status("Device \"%s\" not found", android_serial.c_str());
-
-adb.SetDeviceID(*find_it);
+adb.SetDeviceID(android_serial);
   }
   return error;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 34a57dc - [lldb/Reproducers] Make DoConnectRemote connect to the replay server.

2020-05-07 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-05-07T10:09:09-07:00
New Revision: 34a57dc972c2d16f176e47339a7c2c95cdc31e75

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

LOG: [lldb/Reproducers] Make DoConnectRemote connect to the replay server.

All entry points into ProcessGDBRemote that connect to the debug server
should connect to the replay server instead during reproducer replay.
This patch adds the necessary logic for ConnectRemote, which is
accessible from the SB API. This fixes active replay for
TestRecognizeBreakpoint.py as described in D78588.

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index d2e7f4d479cb..dbad49d62097 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -649,7 +649,10 @@ Status ProcessGDBRemote::DoConnectRemote(Stream *strm,
   if (error.Fail())
 return error;
 
-  error = ConnectToDebugserver(remote_url);
+  if (repro::Loader *loader = repro::Reproducer::Instance().GetLoader())
+error = ConnectToReplayServer(loader);
+  else
+error = ConnectToDebugserver(remote_url);
 
   if (error.Fail())
 return error;



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


[Lldb-commits] [PATCH] D79490: tab completion for register read/write

2020-05-07 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa14f4a7531f0: tab completion for register read/write 
(authored by Gongyu Deng gy_d...@icloud.com, committed by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79490

Files:
  lldb/include/lldb/Interpreter/CommandCompletions.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py

Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -414,3 +414,66 @@
 # No completion for Qu because the candidate is
 # (anonymous namespace)::Quux().
 self.complete_from_to('breakpoint set -n Qu', '')
+
+@skipIf(archs=no_match(['x86_64']))
+def test_register_read_and_write_on_x86(self):
+"""Test the completion of the commands register read and write on x86"""
+
+# The tab completion for "register read/write"  won't work without a running process.
+self.complete_from_to('register read ',
+  'register read ')
+self.complete_from_to('register write ',
+  'register write ')
+
+self.build()
+self.main_source_spec = lldb.SBFileSpec("main.cpp")
+lldbutil.run_to_source_breakpoint(self, '// Break here', self.main_source_spec)
+
+# test cases for register read
+self.complete_from_to('register read ',
+  ['rax',
+   'rbx',
+   'rcx'])
+self.complete_from_to('register read r',
+  ['rax',
+   'rbx',
+   'rcx'])
+self.complete_from_to('register read ra',
+  'register read rax')
+# register read can take multiple register names as arguments
+self.complete_from_to('register read rax ',
+  ['rax',
+   'rbx',
+   'rcx'])
+# complete with prefix '$'
+self.completions_match('register read $rb',
+  ['$rbx',
+   '$rbp'])
+self.completions_match('register read $ra',
+  ['$rax'])
+self.complete_from_to('register read rax $',
+  ['\$rax',
+   '\$rbx',
+   '\$rcx'])
+self.complete_from_to('register read $rax ',
+  ['rax',
+   'rbx',
+   'rcx'])
+
+# test cases for register write
+self.complete_from_to('register write ',
+  ['rax',
+   'rbx',
+   'rcx'])
+self.complete_from_to('register write r',
+  ['rax',
+   'rbx',
+   'rcx'])
+self.complete_from_to('register write ra',
+  'register write rax')
+self.complete_from_to('register write rb',
+  ['rbx',
+   'rbp'])
+# register write can only take exact one register name as argument
+self.complete_from_to('register write rbx ',
+  [])
Index: lldb/source/Commands/CommandObjectRegister.cpp
===
--- lldb/source/Commands/CommandObjectRegister.cpp
+++ lldb/source/Commands/CommandObjectRegister.cpp
@@ -70,6 +70,17 @@
 
   ~CommandObjectRegisterRead() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest ,
+   OptionElementVector _element_vector) override {
+if (!m_exe_ctx.HasProcessScope())
+  return;
+
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), CommandCompletions::eRegisterCompletion,
+request, nullptr);
+  }
+
   Options *GetOptions() override { return _option_group; }
 
   bool DumpRegister(const ExecutionContext _ctx, Stream ,
@@ -323,6 +334,17 @@
 
   ~CommandObjectRegisterWrite() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest ,
+   OptionElementVector _element_vector) override {
+if (!m_exe_ctx.HasProcessScope() || request.GetCursorIndex() != 0)
+  return;
+
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), 

[Lldb-commits] [PATCH] D79563: [lldb/test] Make "inline" tests handle multiple statements at the same location

2020-05-07 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment.

Thank you! I am pleasantly surprised there was only one hidden broken test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79563



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


[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added a comment.

I have to agree w/ Pavel here that I am not crazy about creating creating an 
empty struct here, it is not clear why there is no other way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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


[Lldb-commits] [lldb] a14f4a7 - tab completion for register read/write

2020-05-07 Thread Raphael Isemann via lldb-commits

Author: Gongyu Deng
Date: 2020-05-07T18:14:27+02:00
New Revision: a14f4a7531f083a4820b5452808a1d003f1e88cc

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

LOG: tab completion for register read/write

Summary:
1. Created a new common completion for the registers of the current context;
2. Apply this new common completion to the commands register read/write;
3. Unit test.

Reviewers: teemperor, JDevlieghere

Reviewed By: teemperor

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandCompletions.h
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Commands/CommandObjectRegister.cpp
lldb/test/API/functionalities/completion/TestCompletion.py

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandCompletions.h 
b/lldb/include/lldb/Interpreter/CommandCompletions.h
index 51f5a2e5705e..a6e025e72baf 100644
--- a/lldb/include/lldb/Interpreter/CommandCompletions.h
+++ b/lldb/include/lldb/Interpreter/CommandCompletions.h
@@ -34,10 +34,11 @@ class CommandCompletions {
 ePlatformPluginCompletion = (1u << 6),
 eArchitectureCompletion = (1u << 7),
 eVariablePathCompletion = (1u << 8),
+eRegisterCompletion = (1u << 9),
 // This item serves two purposes.  It is the last element in the enum, so
 // you can add custom enums starting from here in your Option class. Also
 // if you & in this bit the base code will not process the option.
-eCustomCompletion = (1u << 9)
+eCustomCompletion = (1u << 10)
   };
 
   static bool InvokeCommonCompletionCallbacks(
@@ -81,6 +82,9 @@ class CommandCompletions {
 
   static void VariablePath(CommandInterpreter ,
CompletionRequest , SearchFilter *searcher);
+
+  static void Registers(CommandInterpreter ,
+CompletionRequest , SearchFilter *searcher);
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Commands/CommandCompletions.cpp 
b/lldb/source/Commands/CommandCompletions.cpp
index bc3a2ec4bd9e..1e903157c511 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Variable.h"
+#include "lldb/Target/RegisterContext.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/TildeExpressionResolver.h"
@@ -55,6 +56,7 @@ bool CommandCompletions::InvokeCommonCompletionCallbacks(
   {ePlatformPluginCompletion, CommandCompletions::PlatformPluginNames},
   {eArchitectureCompletion, CommandCompletions::ArchitectureNames},
   {eVariablePathCompletion, CommandCompletions::VariablePath},
+  {eRegisterCompletion, CommandCompletions::Registers},
   {eNoCompletion, nullptr} // This one has to be last in the list.
   };
 
@@ -531,3 +533,20 @@ void CommandCompletions::VariablePath(CommandInterpreter 
,
   SearchFilter *searcher) {
   Variable::AutoComplete(interpreter.GetExecutionContext(), request);
 }
+
+void CommandCompletions::Registers(CommandInterpreter ,
+   CompletionRequest ,
+   SearchFilter *searcher) {
+  std::string reg_prefix = "";
+  if (request.GetCursorArgumentPrefix().startswith("$"))
+reg_prefix = "$";
+
+  RegisterContext *reg_ctx =
+  interpreter.GetExecutionContext().GetRegisterContext();
+  const size_t reg_num = reg_ctx->GetRegisterCount();
+  for (size_t reg_idx = 0; reg_idx < reg_num; ++reg_idx) {
+const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoAtIndex(reg_idx);
+request.TryCompleteCurrentArg(reg_prefix + reg_info->name,
+  reg_info->alt_name);
+  }
+}
\ No newline at end of file

diff  --git a/lldb/source/Commands/CommandObjectRegister.cpp 
b/lldb/source/Commands/CommandObjectRegister.cpp
index 2c5457396eca..56e8c3fb4b84 100644
--- a/lldb/source/Commands/CommandObjectRegister.cpp
+++ b/lldb/source/Commands/CommandObjectRegister.cpp
@@ -70,6 +70,17 @@ class CommandObjectRegisterRead : public CommandObjectParsed 
{
 
   ~CommandObjectRegisterRead() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest ,
+   OptionElementVector _element_vector) override {
+if (!m_exe_ctx.HasProcessScope())
+  return;
+
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), CommandCompletions::eRegisterCompletion,
+request, nullptr);
+  }
+
   Options *GetOptions() override { return _option_group; }
 
   bool DumpRegister(const ExecutionContext _ctx, Stream ,
@@ -323,6 +334,17 @@ 

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib marked 3 inline comments as done.
mib added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:665
   bool child_is_deref_of_parent = false;
+  CompilerType compiler_type = GetCompilerType();
   uint64_t language_flags = 0;

teemperor wrote:
> This change (and the one below) don't seem to be some NFC refactoring? Not 
> sure why this was refactored as `compiler_type` is only ever used once where 
> we previously called `GetCompilerType()`?
This is a remaining from a previous change that I ended up not removing. I'll 
change it back the way it was.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py:59-60
+
+self.expect(
+'frame variable -d run-target *cfDictionaryRef',
+patterns=[

friss wrote:
> I must be missing something obvious, but it seems like that patch doesn't 
> register a formatter for CFDictionaryRef. Was it already there but 
> non-functional?
The string summary worked but not the synthetic child provider.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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


[Lldb-commits] [lldb] d7c2c2e - [lldb][NFC] Also initialize language_flags in ValueObject::Dereference

2020-05-07 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-05-07T17:52:12+02:00
New Revision: d7c2c2ed79abd5446f8b485fe8249e1d3b78488d

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

LOG: [lldb][NFC] Also initialize language_flags in ValueObject::Dereference

We currently rely on the TypeSystem implementation to initialize this value
with 0 in the GetChildCompilerTypeAtIndex call below. Let's just initialize
this variable like the rest.

Added: 


Modified: 
lldb/source/Core/ValueObject.cpp

Removed: 




diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 9e20ba76dccb..f80e86fc195b 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -2810,7 +2810,7 @@ ValueObjectSP ValueObject::Dereference(Status ) {
 const bool transparent_pointers = false;
 CompilerType compiler_type = GetCompilerType();
 CompilerType child_compiler_type;
-uint64_t language_flags;
+uint64_t language_flags = 0;
 
 ExecutionContext exe_ctx(GetExecutionContextRef());
 



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


[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

I don't really know that ValueObject code, but I wish we could just pretend 
that the dereferenced type of X is a type named Y or something like that and 
then just map *Ref classes to their non-opaque bridged types when dereferenced. 
No idea if that's possible though.

Anyway, if we go with the fake empty struct approach then I wish we could just 
use the (fixed?) list 

 of these special bridged structs and hard-code them into some ObjC plugin or 
something like that. If those few selected classes are manually completed as 
empty structs when we find them in the debug info then I think that's a 
manageable workaround to get this running.




Comment at: lldb/source/Core/ValueObject.cpp:665
   bool child_is_deref_of_parent = false;
+  CompilerType compiler_type = GetCompilerType();
   uint64_t language_flags = 0;

This change (and the one below) don't seem to be some NFC refactoring? Not sure 
why this was refactored as `compiler_type` is only ever used once where we 
previously called `GetCompilerType()`?



Comment at: lldb/source/Core/ValueObject.cpp:2853
+ConstString g___lldb_opaque_ptr_type(
+compiler_type.GetTypeName().AsCString());
+

This is the name of the typedef, not the underlying incomplete struct. So this 
creates a new empty record with the name of the typedef which seems weird. If 
we fake that some type is actually empty instead of incomplete, then I think 
the underlying struct makes more sense to emulate. Also this variable name is 
kinda weird with it's `g` prefix even though it's not a global or a static.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py:58
 
+
+self.expect(

Trailing spaces (that Phabricator somehow doesn't properly highlight :/)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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


[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

2020-05-07 Thread Djordje Todorovic via Phabricator via lldb-commits
djtodoro added a comment.

In D79491#2024771 , @labath wrote:

> In D79491#2024647 , @djtodoro wrote:
>
> > Thanks a lot for this!
> >
> > > Nevertheless, I am still interested in making assembly-based tests for 
> > > this (and similar features) because it enables testing scenarios that we 
> > > could not get (reliably or at all) a compiler to produce.
> >
> > I also think this would be more stable if we can make assembler-based tests 
> > (but we'll need to address all archs from {x86_64, arm, aarch64}).
> >  I am just wondering, what are the obstacles for writing the 
> > assembler-based tests? Is it LLDB testing infrastructure or writing tests 
> > itself?
>
>
> A bit of both, maybe. Writing a test which works on a single target (os+arch) 
> was relatively easy (for me, because I've done a lot of this stuff lately, 
> maybe not so easy for others), but the difficulties started when I wanted to 
> make that test run on other oses (which have different asm dialects). I was 
> ok with leaving the test x86-specific, since most developers have an x86 
> machine and there is nothing arch-specific about this functionality. However, 
> I did not want to restrict the test to any single OS, and since this test 
> requires a running program, the asm needed to match what the host expects.
>
> I did manage to ifdef around the asm platform quirks, but I still haven't 
> managed to get the darwin linker to recognize my hand-written dwarf. I am 
> sure this can be fixed (I think it's  because I reduced the input too much), 
> and I do want to try it out, but I am not sure the result will be something 
> we can recommend as a general practice.


I see.. Enabling it only for specific OSes  could be a temp solution, but we 
can discuss about that.

> A different way to address this would be to remove the requirement for a 
> running process. The test doesn't really require that, it just needs a 
> relatively complex static snapshot of the process. A core file is just that, 
> but we currently don't have any good way of creating such core files. If we 
> had that, we could run this test over a core file. That would still be 
> platform specific, but then it wouldn't matter (so much) because it wouldn't 
> be tied to the host platform.

Good idea! A corefile could be solution, and I believe LLDB parses corefiles 
from other architectures very well (as multiarch GDB does).




Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py:5
+
+supported_archs = ["x86_64", "arm", "aarch64"]
+

labath wrote:
> djtodoro wrote:
> > I haven't refreshed the page before submitting my previous comment.
> > 
> > WDYT about adding a decorators-function (e.g. 
> > `skipUnlessEntryValuesSupportedArch`) so we can use it in other tests as 
> > well?
> I think that's a good idea, and that the check can be folded into the 
> existing `skipUnlessHasCallSiteInfo` decorator.
Please note that entry values and call site info are not the same thing. Entry 
values implementation ended up describing target-specific instructions (in 
terms of DWARF operations) that loads values into registers that transfers the 
parameters, and that is used for the entry values printing. Eventually, we will 
have all targets supporting the entry values, but the call site Info is 
something different. The call site info is controlled by the 
`DIFlagAllCallsDescribed` flag, and it is generated by the language front-end, 
according to DWARF standard. Therefore, it makes sense to me to add new 
independent function (`skipUnlessEntryValuesSupportedArch`), but the whole 
feature depends on both `skipUnlessEntryValuesSupportedArch` and 
`skipUnlessHasCallSiteInfo`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491



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


[Lldb-commits] [PATCH] D79563: [lldb/test] Make "inline" tests handle multiple statements at the same location

2020-05-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.

Will this also fix these problems?

  int i = 0;
  //% code_that_is_executed
  
  //% that that isn't executed as it's behind an empty line :(

I think disabling the check is fine. You can also just change the `'= 14'` to a 
`'= 11'`. I originally only intended to capture the current behavior and 
assumed we prefer local variables, but the agreement back then was that we 
don't know what is the correct behavior in this situation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79563



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


[Lldb-commits] [PATCH] D79563: [lldb/test] Make "inline" tests handle multiple statements at the same location

2020-05-07 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

Changing might actually be a better idea as this way we at least know this 
isn't crashing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79563



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


[Lldb-commits] [PATCH] D79563: [lldb/test] Make "inline" tests handle multiple statements at the same location

2020-05-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a reviewer: teemperor.
labath marked an inline comment as done.
labath added inline comments.



Comment at: lldb/test/API/commands/expression/dollar-in-variable/main.c:20
   //%self.expect("expr int $foo = 123", error=True, substrs=["declaration 
conflicts"])
-  return 0; //%self.expect("expr $0", substrs=['(int)', ' = 14'])
+  return 0; //TODO: self.expect("expr $0", substrs=['(int)', ' = 14'])
 }

Raphael: This check wasn't executed before this patch. As far as I can tell, 
this functionality hasn't worked since the test was first introduced (r365698).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79563



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


[Lldb-commits] [PATCH] D79563: [lldb/test] Make "inline" tests handle multiple statements at the same location

2020-05-07 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 262614.
labath added a comment.

- disable a check that fails now. AFAICT, this never worked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79563

Files:
  lldb/packages/Python/lldbsuite/test/lldbinline.py
  lldb/test/API/commands/expression/dollar-in-variable/main.c


Index: lldb/test/API/commands/expression/dollar-in-variable/main.c
===
--- lldb/test/API/commands/expression/dollar-in-variable/main.c
+++ lldb/test/API/commands/expression/dollar-in-variable/main.c
@@ -17,5 +17,5 @@
   //%self.expect("expr $foo", substrs=['(int)', ' = 12'])
   //%self.expect("expr $R0", substrs=['(int)', ' = 13'])
   //%self.expect("expr int $foo = 123", error=True, substrs=["declaration 
conflicts"])
-  return 0; //%self.expect("expr $0", substrs=['(int)', ' = 14'])
+  return 0; //TODO: self.expect("expr $0", substrs=['(int)', ' = 14'])
 }
Index: lldb/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/packages/Python/lldbsuite/test/lldbinline.py
@@ -126,6 +126,13 @@
 def execute_user_command(self, __command):
 exec(__command, globals(), locals())
 
+def _get_breakpoint_ids(self, thread):
+ids = set()
+for i in range(0, thread.GetStopReasonDataCount(), 2):
+ids.add(thread.GetStopReasonDataAtIndex(i))
+self.assertGreater(len(ids), 0)
+return sorted(ids)
+
 def do_test(self):
 exe = self.getBuildArtifact("a.out")
 source_files = [f for f in os.listdir(self.getSourceDir())
@@ -145,8 +152,8 @@
 hit_breakpoints += 1
 thread = lldbutil.get_stopped_thread(
 process, lldb.eStopReasonBreakpoint)
-breakpoint_id = thread.GetStopReasonDataAtIndex(0)
-parser.handle_breakpoint(self, breakpoint_id)
+for id in self._get_breakpoint_ids(thread):
+parser.handle_breakpoint(self, id)
 process.Continue()
 
 self.assertTrue(hit_breakpoints > 0,


Index: lldb/test/API/commands/expression/dollar-in-variable/main.c
===
--- lldb/test/API/commands/expression/dollar-in-variable/main.c
+++ lldb/test/API/commands/expression/dollar-in-variable/main.c
@@ -17,5 +17,5 @@
   //%self.expect("expr $foo", substrs=['(int)', ' = 12'])
   //%self.expect("expr $R0", substrs=['(int)', ' = 13'])
   //%self.expect("expr int $foo = 123", error=True, substrs=["declaration conflicts"])
-  return 0; //%self.expect("expr $0", substrs=['(int)', ' = 14'])
+  return 0; //TODO: self.expect("expr $0", substrs=['(int)', ' = 14'])
 }
Index: lldb/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/packages/Python/lldbsuite/test/lldbinline.py
@@ -126,6 +126,13 @@
 def execute_user_command(self, __command):
 exec(__command, globals(), locals())
 
+def _get_breakpoint_ids(self, thread):
+ids = set()
+for i in range(0, thread.GetStopReasonDataCount(), 2):
+ids.add(thread.GetStopReasonDataAtIndex(i))
+self.assertGreater(len(ids), 0)
+return sorted(ids)
+
 def do_test(self):
 exe = self.getBuildArtifact("a.out")
 source_files = [f for f in os.listdir(self.getSourceDir())
@@ -145,8 +152,8 @@
 hit_breakpoints += 1
 thread = lldbutil.get_stopped_thread(
 process, lldb.eStopReasonBreakpoint)
-breakpoint_id = thread.GetStopReasonDataAtIndex(0)
-parser.handle_breakpoint(self, breakpoint_id)
+for id in self._get_breakpoint_ids(thread):
+parser.handle_breakpoint(self, id)
 process.Continue()
 
 self.assertTrue(hit_breakpoints > 0,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

2020-05-07 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done.
labath added inline comments.



Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py:5
+
+supported_archs = ["x86_64", "arm", "aarch64"]
+

djtodoro wrote:
> I haven't refreshed the page before submitting my previous comment.
> 
> WDYT about adding a decorators-function (e.g. 
> `skipUnlessEntryValuesSupportedArch`) so we can use it in other tests as well?
I think that's a good idea, and that the check can be folded into the existing 
`skipUnlessHasCallSiteInfo` decorator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491



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


[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

2020-05-07 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done.
labath added a comment.

In D79491#2024647 , @djtodoro wrote:

> Thanks a lot for this!
>
> > Nevertheless, I am still interested in making assembly-based tests for this 
> > (and similar features) because it enables testing scenarios that we could 
> > not get (reliably or at all) a compiler to produce.
>
> I also think this would be more stable if we can make assembler-based tests 
> (but we'll need to address all archs from {x86_64, arm, aarch64}).
>  I am just wondering, what are the obstacles for writing the assembler-based 
> tests? Is it LLDB testing infrastructure or writing tests itself?


A bit of both, maybe. Writing a test which works on a single target (os+arch) 
was relatively easy (for me, because I've done a lot of this stuff lately, 
maybe not so easy for others), but the difficulties started when I wanted to 
make that test run on other oses (which have different asm dialects). I was ok 
with leaving the test x86-specific, since most developers have an x86 machine 
and there is nothing arch-specific about this functionality. However, I did not 
want to restrict the test to any single OS, and since this test requires a 
running program, the asm needed to match what the host expects.

I did manage to ifdef around the asm platform quirks, but I still haven't 
managed to get the darwin linker to recognize my hand-written dwarf. I am sure 
this can be fixed (I think it's  because I reduced the input too much), and I 
do want to try it out, but I am not sure the result will be something we can 
recommend as a general practice.

A different way to address this would be to remove the requirement for a 
running process. The test doesn't really require that, it just needs a 
relatively complex static snapshot of the process. A core file is just that, 
but we currently don't have any good way of creating such core files. If we had 
that, we could run this test over a core file. That would still be platform 
specific, but then it wouldn't matter (so much) because it wouldn't be tied to 
the host platform.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491



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


[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

2020-05-07 Thread Djordje Todorovic via Phabricator via lldb-commits
djtodoro added inline comments.



Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py:5
+
+supported_archs = ["x86_64", "arm", "aarch64"]
+

I haven't refreshed the page before submitting my previous comment.

WDYT about adding a decorators-function (e.g. 
`skipUnlessEntryValuesSupportedArch`) so we can use it in other tests as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491



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


[Lldb-commits] [PATCH] D79563: [lldb/test] Make "inline" tests handle multiple statements at the same location

2020-05-07 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: vsk, JDevlieghere.
Herald added a project: LLDB.

The test machinery translates each continuous block of "//%" comments
into a single breakpoint. If there's no code between the blocks the
breakpoints will end up at the same location in the program. When the
process stops at a breakpoint lldb correctly reports all breakpoint IDs,
but the test machinery only looks at the first one. This results in a
very dangerous situation as it means some checks can be silently
stopped.

This patch fixes that by making the test machinery iterate through all
breakpoints at a given location and execute all commands.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79563

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


Index: lldb/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/packages/Python/lldbsuite/test/lldbinline.py
@@ -126,6 +126,13 @@
 def execute_user_command(self, __command):
 exec(__command, globals(), locals())
 
+def _get_breakpoint_ids(self, thread):
+ids = set()
+for i in range(0, thread.GetStopReasonDataCount(), 2):
+ids.add(thread.GetStopReasonDataAtIndex(i))
+self.assertGreater(len(ids), 0)
+return sorted(ids)
+
 def do_test(self):
 exe = self.getBuildArtifact("a.out")
 source_files = [f for f in os.listdir(self.getSourceDir())
@@ -145,8 +152,8 @@
 hit_breakpoints += 1
 thread = lldbutil.get_stopped_thread(
 process, lldb.eStopReasonBreakpoint)
-breakpoint_id = thread.GetStopReasonDataAtIndex(0)
-parser.handle_breakpoint(self, breakpoint_id)
+for id in self._get_breakpoint_ids(thread):
+parser.handle_breakpoint(self, id)
 process.Continue()
 
 self.assertTrue(hit_breakpoints > 0,


Index: lldb/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/packages/Python/lldbsuite/test/lldbinline.py
@@ -126,6 +126,13 @@
 def execute_user_command(self, __command):
 exec(__command, globals(), locals())
 
+def _get_breakpoint_ids(self, thread):
+ids = set()
+for i in range(0, thread.GetStopReasonDataCount(), 2):
+ids.add(thread.GetStopReasonDataAtIndex(i))
+self.assertGreater(len(ids), 0)
+return sorted(ids)
+
 def do_test(self):
 exe = self.getBuildArtifact("a.out")
 source_files = [f for f in os.listdir(self.getSourceDir())
@@ -145,8 +152,8 @@
 hit_breakpoints += 1
 thread = lldbutil.get_stopped_thread(
 process, lldb.eStopReasonBreakpoint)
-breakpoint_id = thread.GetStopReasonDataAtIndex(0)
-parser.handle_breakpoint(self, breakpoint_id)
+for id in self._get_breakpoint_ids(thread):
+parser.handle_breakpoint(self, id)
 process.Continue()
 
 self.assertTrue(hit_breakpoints > 0,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

2020-05-07 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 262595.
labath marked an inline comment as done.
labath added a comment.

- make the test architecture-independent (and rename accordingly)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491

Files:
  lldb/test/API/functionalities/param_entry_vals/basic_entry_values/Makefile
  
lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py
  lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
  
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/Makefile
  
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
  
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp

Index: lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
===
--- lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
+++ /dev/null
@@ -1,14 +0,0 @@
-from lldbsuite.test import lldbinline
-from lldbsuite.test import decorators
-from lldbsuite.test import lldbplatformutil
-
-supported_platforms = ["linux"]
-supported_platforms.extend(lldbplatformutil.getDarwinOSTriples())
-
-lldbinline.MakeInlineTest(__file__, globals(),
-[decorators.skipIf(bugnumber="llvm.org/PR44774"),
- decorators.skipUnlessPlatform(supported_platforms),
- decorators.skipIf(compiler="clang", compiler_version=['<', '10.0']),
- decorators.skipUnlessArch('x86_64'),
- decorators.skipUnlessHasCallSiteInfo,
- decorators.skipIf(dwarf_version=['<', '4'])])
Index: lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
===
--- lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
+++ lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp
@@ -1,105 +1,61 @@
-// Note: This test requires the SysV AMD64 ABI to be in use, and requires
-// compiler support for DWARF entry values.
-
-// Inhibit dead-arg-elim by using 'x'.
-template __attribute__((noinline)) void use(T x) {
-  asm volatile (""
-  /* Outputs */  :
-  /* Inputs */   : "g"(x)
-  /* Clobbers */ :
-  );
-}
+// Note: This test requires compiler support for DWARF entry values.
+
+int dummy;
+volatile int global = 0;
 
-// Destroy %rsi in the current frame.
-#define DESTROY_RSI \
-  asm volatile ("xorq %%rsi, %%rsi" \
-  /* Outputs */  : \
-  /* Inputs */   : \
-  /* Clobbers */ : "rsi" \
-  );
-
-// Destroy %rbx in the current frame.
-#define DESTROY_RBX \
-  asm volatile ("xorq %%rbx, %%rbx" \
-  /* Outputs */  : \
-  /* Inputs */   : \
-  /* Clobbers */ : "rbx" \
-  );
+template  __attribute__((optnone)) void use(T...) {}
 
 struct S1 {
   int field1 = 123;
   int *field2 = 
 };
 
-__attribute__((noinline))
-void func1(int , int x) {
-  use(x);
-
-  // Destroy 'x' in the current frame.
-  DESTROY_RSI;
-
-  // NOTE: Currently, we do not generate DW_OP_entry_value for the 'x',
-  // since it gets copied into a register that is not callee saved,
-  // and we can not guarantee that its value has not changed.
-
-  ++sink;
-
-  // Destroy 'sink' in the current frame.
-  DESTROY_RBX;
+__attribute__((noinline)) void func1(int ) {
+  // First use works around a compiler "bug" where an unused variable gets
+  // no location descriptions. The second use overwrites the function arguments
+  // with other values.
+  use(sink);
+  use(dummy);
 
+  ++global;
   //% self.filecheck("image lookup -va $pc", "main.cpp", "-check-prefix=FUNC1-DESC")
-  // FUNC1-DESC: name = "sink", type = "int &", location = DW_OP_entry_value(DW_OP_reg5 RDI)
+  // FUNC1-DESC: name = "sink", type = "int &", location = DW_OP_entry_value
 }
 
 __attribute__((noinline))
 void func2(int , int x) {
-  use(x);
-
-  // Destroy 'x' in the current frame.
-  DESTROY_RSI;
-
-  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC2-EXPR-FAIL", expect_cmd_failure=True)
-  // FUNC2-EXPR-FAIL: couldn't get the value of variable x: variable not available
-
-  ++sink;
-
-  // Destroy 'sink' in the current frame.
-  DESTROY_RBX;
-
-  //% self.filecheck("expr sink", "main.cpp", "-check-prefix=FUNC2-EXPR")
-  // FUNC2-EXPR: ${{.*}} = 2
+  use(sink, x);
+  use(dummy, 0);
+
+  ++global;
+  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC2-EXPR1")
+  //% self.filecheck("expr sink", "main.cpp", "-check-prefix=FUNC2-EXPR2")
+  // FUNC2-EXPR1: ${{.*}} = 123
+  // FUNC2-EXPR2: ${{.*}} = 2
 }
 
 __attribute__((noinline))
 void func3(int , int *p) {
-  use(p);
-
-  // Destroy 'p' in the current frame.
-  DESTROY_RSI;
+  use(sink, p);
+  use(dummy, nullptr);
 
   //% self.filecheck("expr *p", "main.cpp", "-check-prefix=FUNC3-EXPR")
   // 

[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

2020-05-07 Thread Pavel Labath via Phabricator via lldb-commits
labath marked 6 inline comments as done.
labath added inline comments.



Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:21-22
+  ++global;
   //% self.filecheck("image lookup -va $pc", "main.cpp", 
"-check-prefix=FUNC1-DESC")
   // FUNC1-DESC: name = "sink", type = "int &", location = 
DW_OP_entry_value(DW_OP_reg5 RDI)
 }

vsk wrote:
> labath wrote:
> > If we remove this check, the test will be completely architecture- and 
> > abi-independent. I don't think this check is particularly useful (we use 
> > llvm to print the dwarf expression, and there are better ways to test the 
> > image lookup command). Maybe we could just keep it to ensure that we really 
> > are evaluating entry values, but change the check the just search for the 
> > DW_OP_entry_value keyword (and then run the test on all architectures)?
> We should stop matching %rdi, as the purpose of the check is just to 
> determine whether we really are testing entry value evaluation. However, llvm 
> doesn't support entry value production for all platforms, so we would need to 
> restrict the test to {x86_64, arm, aarch64} (still a clear improvement over 
> the current situation).
Sounds good. (I'm not sure we even have functioning bots for non-x86, non-arm 
platforms).



Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:34
+  // FUNC2-EXPR1: ${{.*}} = 123
+  // FUNC2-EXPR2: ${{.*}} = 2
 }

vsk wrote:
> Hm, I thought inline tests only supported running one command per breakpoint. 
> Have you tried running this test with `--param dotest-args='-t'` to verify 
> that FUNC2-EXPR2 gets matched? If it does, that's great; if not, we can 
> manufacture a second breakpoint by adding another '++global' or convert this 
> to an API test.
I've verified that this works. The trick is that the `//%` lines need to be in 
a single continuous block. (The restriction not based on commands/statements, 
but text blocks -- the inline machinery does not understand where python 
statements end).

I made a note to my self to see if this restriction can be lifted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491



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


[Lldb-commits] [PATCH] D79491: [lldb] Revive TestBasicEntryValuesX86_64

2020-05-07 Thread Djordje Todorovic via Phabricator via lldb-commits
djtodoro added a comment.

Thanks a lot for this!

> Nevertheless, I am still interested in making assembly-based tests for this 
> (and similar features) because it enables testing scenarios that we could not 
> get (reliably or at all) a compiler to produce.

I also think this would be more stable if we can make assembler-based tests 
(but we'll need to address all archs from {x86_64, arm, aarch64}).
I am just wondering, what are the obstacles for writing the assembler-based 
tests? Is it LLDB testing infrastructure or writing tests itself?




Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:21-22
+  ++global;
   //% self.filecheck("image lookup -va $pc", "main.cpp", 
"-check-prefix=FUNC1-DESC")
   // FUNC1-DESC: name = "sink", type = "int &", location = 
DW_OP_entry_value(DW_OP_reg5 RDI)
 }

labath wrote:
> vsk wrote:
> > labath wrote:
> > > If we remove this check, the test will be completely architecture- and 
> > > abi-independent. I don't think this check is particularly useful (we use 
> > > llvm to print the dwarf expression, and there are better ways to test the 
> > > image lookup command). Maybe we could just keep it to ensure that we 
> > > really are evaluating entry values, but change the check the just search 
> > > for the DW_OP_entry_value keyword (and then run the test on all 
> > > architectures)?
> > We should stop matching %rdi, as the purpose of the check is just to 
> > determine whether we really are testing entry value evaluation. However, 
> > llvm doesn't support entry value production for all platforms, so we would 
> > need to restrict the test to {x86_64, arm, aarch64} (still a clear 
> > improvement over the current situation).
> Sounds good. (I'm not sure we even have functioning bots for non-x86, non-arm 
> platforms).
>We should stop matching %rdi, as the purpose of the check is just to determine 
>whether we really are testing entry value evaluation. However, llvm doesn't 
>support entry value production for all platforms, so we would need to restrict 
>the test to {x86_64, arm, aarch64} (still a clear improvement over the current 
>situation).
+1
We can teach the `TestBasicEntryValuesX86_64.py` to keep arm and aarch64 as 
well.
Furthermore, we can add a function into decorators something like 
`skipUnlessEntryValuesSupportedArch` checking if the arch is in {x86_64, arm, 
aarch64}.



Comment at: 
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp:61-62
-
-  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC2-EXPR-FAIL", 
expect_cmd_failure=True)
-  // FUNC2-EXPR-FAIL: couldn't get the value of variable x: variable not 
available
-

vsk wrote:
> labath wrote:
> > I'm not sure why this was previously expected to fail -- I don't see a 
> > reason why the compiler couldn't emit an entry value for `x` now, or before 
> > this patch. And in the new setup, the expression actually succeeds.
> IIRC this is left over from when entry value propagation in LiveDebugValues 
> was being reworked.
Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79491



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


[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a reviewer: teemperor.
labath added a comment.

The ValueObject change looks very worrying to me. The ability to pretty-print 
incomplete types seems pretty useful, but the implementation of that looks a 
bit... scary. I mean you're not even checking whether the incomplete type is a 
struct or an enum. And the effect of forcefully creating a empty struct has 
impact outside of the pretty-printer. Maybe it would be better if we thought 
the valueobject/pretty-printer machinery to work with incomplete types?

Raphael, what do you make of all of this?

Also would it be possible to test the ValueObject functionality on a c++ test 
(an incomplete c++ type with a custom pretty printer), so that test can run 
every where?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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


[Lldb-commits] [PATCH] D79559: [lldb] Also recognize DWARF UTF base types using their size

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

Thanks for the patch.

In D79559#2024573 , @Geod24 wrote:

> I wasn't sure what would be the best place / how to add an unit test for 
> this. Could a reviewer provide some pointers ?


I think the simplest approach would be to create some assembly representing a 
compile unit which declares a couple of global variables of this type and then 
run "lldb -o "target variable var1 var2..." over the compiled object. You can 
look at some of the files in `test/Shell/SymbolFile/DWARF` (e.g. 
`DW_OP_piece-struct.s`) for inspiration. Using llvm IR instead of assembly 
would work too, assuming you can represent this thing in IR..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79559



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


[Lldb-commits] [PATCH] D79559: [lldb] Also recognize DWARF UTF base types using their size

2020-05-07 Thread Mathias LANG via Phabricator via lldb-commits
Geod24 added a comment.

I wasn't sure what would be the best place / how to add an unit test for this. 
Could a reviewer provide some pointers ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79559



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


[Lldb-commits] [PATCH] D79559: [lldb] Also recognize DWARF UTF base types using their size

2020-05-07 Thread Mathias LANG via Phabricator via lldb-commits
Geod24 created this revision.
Geod24 added a reviewer: clayborg.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Geod24 added a comment.

I wasn't sure what would be the best place / how to add an unit test for this. 
Could a reviewer provide some pointers ?


The D programming language has 'char', 'wchar', and 'dchar' as base types,
which are defined as UTF-8, UTF-16, and UTF-32, respectively.

It also has type constructors (e.g. 'const' and 'immutable'),
that leads to D compilers emitting DW_TAG_base_type with DW_ATE_UTF
and name 'char', 'immutable(wchar)', 'const(char)', etc...

Before this patch, DW_ATE_UTF would only recognize types that
followed the C/C++ naming, and emit an error message for the rest, e.g.:

  error: need to add support for DW_TAG_base_type 'immutable(char)'
  encoded with DW_ATE = 0x10, bit_size = 8

The code was changed to check the byte size first,
then fall back to the old name-based check.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79559

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp


Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1130,13 +1130,22 @@
 break;
 
   case DW_ATE_UTF:
-if (!type_name.empty()) {
-  if (type_name == "char16_t")
-return GetType(ast.Char16Ty);
-  if (type_name == "char32_t")
-return GetType(ast.Char32Ty);
-  if (type_name == "char8_t")
-return GetType(ast.Char8Ty);
+switch (bit_size) {
+case 8:
+  return GetType(ast.Char8Ty);
+case 16:
+  return GetType(ast.Char16Ty);
+case 32:
+  return GetType(ast.Char32Ty);
+default:
+  if (!type_name.empty()) {
+if (type_name == "char16_t")
+  return GetType(ast.Char16Ty);
+if (type_name == "char32_t")
+  return GetType(ast.Char32Ty);
+if (type_name == "char8_t")
+  return GetType(ast.Char8Ty);
+  }
 }
 break;
   }


Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1130,13 +1130,22 @@
 break;
 
   case DW_ATE_UTF:
-if (!type_name.empty()) {
-  if (type_name == "char16_t")
-return GetType(ast.Char16Ty);
-  if (type_name == "char32_t")
-return GetType(ast.Char32Ty);
-  if (type_name == "char8_t")
-return GetType(ast.Char8Ty);
+switch (bit_size) {
+case 8:
+  return GetType(ast.Char8Ty);
+case 16:
+  return GetType(ast.Char16Ty);
+case 32:
+  return GetType(ast.Char32Ty);
+default:
+  if (!type_name.empty()) {
+if (type_name == "char16_t")
+  return GetType(ast.Char16Ty);
+if (type_name == "char32_t")
+  return GetType(ast.Char32Ty);
+if (type_name == "char8_t")
+  return GetType(ast.Char8Ty);
+  }
 }
 break;
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D75607: [lldb] Use llvm::MC for register numbers in AArch64 ABIs

2020-05-07 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a subscriber: clayborg.
omjavaid added a comment.

In D75607#2020365 , @labath wrote:

> Thanks for trying this out. Do you by any chance have any ideas why would 
> lldb assign a dwarf number to the pc register when neither llvm nor the arm64 
> abi do not specify a pc register number?


Not sure why this was added seems like original import from apple repos had 
this but AArch64 procedure call standard doesnt have it as you said. This also 
means compiler should never emit a dwarf instruction with reg num 32. GDB 
doesnt use PC reg num either and i dont remember seeing any special use of this 
register number anywhere in LLDB. May be ask @clayborg or @jasonmolenda  they 
seems to have written the code according to commit log.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75607



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


[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:52
 
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+

You don't want to introduce a dependency between Core and a plugin. What you 
need here might need to be introduced as a new abstract functionality on the 
TypeSystem base class.



Comment at: lldb/source/Core/ValueObject.cpp:2846
+  if (llvm::isa(compiler_type.GetTypeSystem())) {
+if (HasSyntheticValue()) {
+  TargetSP target_sp = GetTargetSP();

I am understanding correctly that this condition will only trigger when there's 
a synthetic child provider for the type? In other words, if we have an opaque 
pointer that we don't know how to display, it will still cause an error, right?



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py:59-60
+
+self.expect(
+'frame variable -d run-target *cfDictionaryRef',
+patterns=[

I must be missing something obvious, but it seems like that patch doesn't 
register a formatter for CFDictionaryRef. Was it already there but 
non-functional?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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


[Lldb-commits] [PATCH] D79308: [lldb-server] Reset stop reason of all threads when resuming

2020-05-07 Thread Jaroslav Sevcik via Phabricator via lldb-commits
jarin added a comment.

Jim, do you think this is good to go?


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

https://reviews.llvm.org/D79308



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