Fix lldb-server test suite for python3

[labath]

Summary

This patch finishes the python3-ification of the lldb-server test suite.
It reverts the partial attempt in r352709 to encode/decode the string
via utf8 before writing to the socket. This wasn't enough because the
gdb-remote protocol can sometimes (but not very often) carry binary
data, and the utf8 codec chokes on that. Instead I add utility functions
to the "seven" module for performing "identity" transformations on the
byte data. This basically drills back the hole in the python type system
that the string/bytes distinction was supposed to plug. That is not
ideal, but was the best solution of the alternatives I could come up
with. The options I considered were:

• make use of the type system to add type safety to the test suite: This required making a lot of changes to the test suite, since most of the strings would now become byte objects instead, and it was not even fully clear to me where to draw the line. One extreme solution would be to just use byte objects everywhere, as the protocol doesn't support non-ascii characters anyway. However, this appeared to be: a) weird, because most of the protocol actually deals with strings, but we would have to prefix everything with 'b' b) clunky, because the handling of the bytes objects is sufficiently different in PY2 and PY3 (e.g. b'a'[0] is a string in PY2, but an int in PY3).
• using the latin1 codec (which gives an identity transformation for the first 256 code points of unicode) instead of the custom bytes_to_string functions. This almost could work, but it was still slightly different between python 2 and 3, because in PY2 in would return a unicode object, which would then cause problems when combined with regular strings if it contained 8-bit chars.

With this in mind, I think the best solution for the time being is to
just coerce everything into the string type as early as possible, and
have things proceed indentically on both python versions. Once we stop
supporting python3, we can revisit the idea of using bytes objects more
prevasively.

[serge-sans-paille]

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

415

Maybe from __future__ import division to make sure the / operator behaves consistently across versions? That way it may uncover missed / to // conversion?

[davide]

LGTM

[stella.stamenova]

packages/Python/lldbsuite/support/seven.py

28

There are other places in the lldb test suite and lit where similar functions are defined. Rather than just adding new ones here, can you normalize them and perhaps use your functions everywhere instead of ending up with two (or three) solutions?

One place that comes to mind is lit\helper\build.py: to_string. There are others as well that implement conversions in-place and usually use isinstance to determine what to do.

[labath]

Thanks for the review Stella. I was hoping someone would step in and tell me that there is a better way to do that. However :), I see two problems with your proposal:

• build.py is a standalone file, and so it is not easy for it to share code with other stuff. This can be solved with some use_lldb_suite magic or moving the script some place else, but then comes the second problem:
• I am not sure these two use cases are actually compatible. Here I specifically want to "reinterpret_cast" binary bytes without doing any sort of encoding. The only place where to_string is used in build.py currently is on a value retrieved from the windows registry. I'm not 100% sure, but it seems to me that using utf8 encoding (which is what to_string is doing) is precisely the right thing to do there.

With that in mind, I propose to do the following:

• leave build.py alone for now
• rename the new functions I'm introducing here to something less generic. Maybe bitcast_to_string and bitcast_to_bytes?

WDYT?

[labath]

• rename the string conversion functions and explain their operation in more detail
• add "from future import division" to ensure consistency between python versions

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

415

That sounds like a good idea. I didn't know about that. Thanks.

[stella.stamenova]

In D58177#1397873, @labath wrote:

Thanks for the review Stella. I was hoping someone would step in and tell me that there is a better way to do that. However :), I see two problems with your proposal:

• build.py is a standalone file, and so it is not easy for it to share code with other stuff. This can be solved with some use_lldb_suite magic or moving the script some place else, but then comes the second problem:
• I am not sure these two use cases are actually compatible. Here I specifically want to "reinterpret_cast" binary bytes without doing any sort of encoding. The only place where to_string is used in build.py currently is on a value retrieved from the windows registry. I'm not 100% sure, but it seems to me that using utf8 encoding (which is what to_string is doing) is precisely the right thing to do there.

With that in mind, I propose to do the following:

• leave build.py alone for now
• rename the new functions I'm introducing here to something less generic. Maybe bitcast_to_string and bitcast_to_bytes?

WDYT?

Thanks, Pavel. That sounds good and it will make it clear that they two functions are distinct.

[labath]

Thank you for reviewing this.

packages/Python/lldbsuite/support/seven.py

+import binascii
 import six
 
 if six.PY2:
     import commands
     get_command_output = commands.getoutput
     get_command_status_output = commands.getstatusoutput
 
     cmp_ = cmp
def get_command_status_output(command):
                     universal_newlines=True).rstrip())
         except subprocess.CalledProcessError as e:
             return (e.returncode, e.output)
 
     def get_command_output(command):
         return get_command_status_output(command)[1]
 
     cmp_ = lambda x, y: (x > y) - (x < y)
+
+def bytes_to_string(b):
+    """Take a string(PY2) or a bytes(PY3) object and return a string."""
+    return b if six.PY2 else b.decode("latin1")
+
+def string_to_bytes(s):
+    """Take a string and return a string(PY2) or a bytes(PY3) object."""
+    return s if six.PY2 else s.encode("latin1")
+
+def unhexlify(hexstr):
+    """Hex-decode a string. The result is always a string."""
+    return bytes_to_string(binascii.unhexlify(hexstr))
+
+def hexlify(data):
+    """Hex-encode string data. The result if always a string."""
+    return bytes_to_string(binascii.hexlify(string_to_bytes(data)))

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

 from __future__ import print_function
 
 
 import gdbremote_testcase
 import lldbgdbserverutils
+from lldbsuite.support import seven
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
 class TestGdbRemoteModuleInfo(gdbremote_testcase.GdbRemoteTestCaseBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     def module_info(self):
         procs = self.prep_debug_monitor_and_inferior()
         self.add_process_info_collection_packets()
         context = self.expect_gdbremote_sequence()
         info = self.parse_process_info_response(context)
 
         self.test_sequence.add_log_lines([
             'read packet: $jModulesInfo:[{"file":"%s","triple":"%s"}]]#00' % (
                 lldbutil.append_to_process_working_directory(self, "a.out"),
-                info["triple"].decode('hex')),
+                seven.unhexlify(info["triple"])),
             {"direction": "send",
              "regex": r'^\$\[{(.*)}\]\]#[0-9A-Fa-f]{2}',
              "capture": {1: "spec"}},
         ], True)
 
         context = self.expect_gdbremote_sequence()
         spec = context.get("spec")
         self.assertRegexpMatches(spec, '"file_path":".*"')

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

 from __future__ import print_function
 
 
 import unittest2
 import gdbremote_testcase
 import lldbgdbserverutils
 import platform
 import signal
+from lldbsuite.support import seven
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test.lldbdwarf import *
 from lldbsuite.test import lldbutil
 
 
 class LldbGdbServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase, DwarfOpcodeParser):
 
def m_packet_reads_memory(self):
             True)
 
         # Run the packet stream.
         context = self.expect_gdbremote_sequence()
         self.assertIsNotNone(context)
 
         # Ensure what we read from inferior memory is what we wrote.
         self.assertIsNotNone(context.get("read_contents"))
-        read_contents = context.get("read_contents").decode("hex")
+        read_contents = seven.unhexlify(context.get("read_contents"))
         self.assertEqual(read_contents, MEMORY_CONTENTS)
 
     @debugserver_test
     @skipIfDarwinEmbedded # <rdar://problem/34539270> lldb-server tests not updated to work on ios etc yet
     def test_m_packet_reads_memory_debugserver(self):
         self.init_debugserver_test()
         self.build()
         self.set_inferior_startup_launch()
def written_M_content_reads_back_correctly(self):
         context = self.expect_gdbremote_sequence()
         self.assertIsNotNone(context)
 
         # Grab the message address.
         self.assertIsNotNone(context.get("message_address"))
         message_address = int(context.get("message_address"), 16)
 
         # Hex-encode the test message, adding null termination.
-        hex_encoded_message = TEST_MESSAGE.encode("hex")
+        hex_encoded_message = seven.hexlify(TEST_MESSAGE)
 
         # Write the message to the inferior. Verify that we can read it with the hex-encoded (m)
         # and binary (x) memory read packets.
         self.reset_test_sequence()
         self.test_sequence.add_log_lines(
             ["read packet: $M{0:x},{1:x}:{2}#00".format(message_address, len(TEST_MESSAGE), hex_encoded_message),
              "send packet: $OK#00",
              "read packet: $m{0:x},{1:x}#00".format(message_address, len(TEST_MESSAGE)),
def P_and_p_thread_suffix_work(self):
         self.assertIsNotNone(endian)
 
         reg_infos = self.parse_register_info_packets(context)
         self.assertIsNotNone(reg_infos)
         self.add_lldb_register_index(reg_infos)
 
         reg_index = self.select_modifiable_register(reg_infos)
         self.assertIsNotNone(reg_index)
-        reg_byte_size = int(reg_infos[reg_index]["bitsize"]) / 8
+        reg_byte_size = int(reg_infos[reg_index]["bitsize"]) // 8
         self.assertTrue(reg_byte_size > 0)
 
         # Run the process a bit so threads can start up, and collect register
         # info.
         context = self.run_process_then_stop(run_seconds=1)
         self.assertIsNotNone(context)
 
         # Wait for 3 threads to be present.

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

 import signal
 import socket
 import subprocess
 import sys
 import tempfile
 import time
 from lldbsuite.test import configuration
 from lldbsuite.test.lldbtest import *
+from lldbsuite.support import seven
 from lldbgdbserverutils import *
 import logging
 
 
 class _ConnectionRefused(IOError):
     pass
 
 
def expect_socket_recv(
         timeout_time = time.time() + timeout_seconds
 
         while not expected_content_regex.match(
                 response) and time.time() < timeout_time:
             can_read, _, _ = select.select([sock], [], [], timeout_seconds)
             if can_read and sock in can_read:
                 recv_bytes = sock.recv(4096)
                 if recv_bytes:
-                    response += recv_bytes.decode("utf-8")
+                    response += seven.bytes_to_string(recv_bytes)
 
         self.assertTrue(expected_content_regex.match(response))
 
     def expect_socket_send(self, sock, content, timeout_seconds):
         request_bytes_remaining = content
         timeout_time = time.time() + timeout_seconds
 
         while len(request_bytes_remaining) > 0 and time.time() < timeout_time:
def flip_all_bits_in_each_register_value(
 
         for reg_info in reg_infos:
             # Use the lldb register index added to the reg info.  We're not necessarily
             # working off a full set of register infos, so an inferred register
             # index could be wrong.
             reg_index = reg_info["lldb_register_index"]
             self.assertIsNotNone(reg_index)
 
-            reg_byte_size = int(reg_info["bitsize"]) / 8
+            reg_byte_size = int(reg_info["bitsize"]) // 8
             self.assertTrue(reg_byte_size > 0)
 
             # Handle thread suffix.
             if thread_id:
                 p_request = "read packet: $p{:x};thread:{:x}#00".format(
                     reg_index, thread_id)
             else:
                 p_request = "read packet: $p{:x}#00".format(reg_index)
def flip_all_bits_in_each_register_value(
 
             # Verify the response length.
             p_response = context.get("p_response")
             self.assertIsNotNone(p_response)
             initial_reg_value = unpack_register_hex_unsigned(
                 endian, p_response)
 
             # Flip the value by xoring with all 1s
-            all_one_bits_raw = "ff" * (int(reg_info["bitsize"]) / 8)
+            all_one_bits_raw = "ff" * (int(reg_info["bitsize"]) // 8)
             flipped_bits_int = initial_reg_value ^ int(all_one_bits_raw, 16)
             # print("reg (index={}, name={}): val={}, flipped bits (int={}, hex={:x})".format(reg_index, reg_info["name"], initial_reg_value, flipped_bits_int, flipped_bits_int))
 
             # Handle thread suffix for P.
             if thread_id:
                 P_request = "read packet: $P{:x}={};thread:{:x}#00".format(
                     reg_index, pack_register_hex(
                         endian, flipped_bits_int, byte_size=reg_byte_size), thread_id)
def g_c1_c2_contents_are(self, args):
         # Run the packet stream.
         context = self.expect_gdbremote_sequence()
         self.assertIsNotNone(context)
 
         # Check if what we read from inferior memory is what we are expecting.
         self.assertIsNotNone(context.get("g_c1_contents"))
         self.assertIsNotNone(context.get("g_c2_contents"))
 
-        return (context.get("g_c1_contents").decode("hex") == expected_g_c1) and (
-            context.get("g_c2_contents").decode("hex") == expected_g_c2)
+        return (seven.unhexlify(context.get("g_c1_contents")) == expected_g_c1) and (
+            seven.unhexlify(context.get("g_c2_contents")) == expected_g_c2)
 
     def single_step_only_steps_one_instruction(
             self, use_Hc_packet=True, step_instruction="s"):
         """Used by single step test that appears in a few different contexts."""
         # Start up the inferior.
         procs = self.prep_debug_monitor_and_inferior(
             inferior_args=[
                 "get-code-address-hex:swap_chars",

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

def pack_register_hex(endian, value, byte_size=None):
     if endian == 'little':
         # Create the litt-endian return value.
         retval = ""
         while value != 0:
             retval = retval + "{:02x}".format(value & 0xff)
             value = value >> 8
         if byte_size:
             # Add zero-fill to the right/end (MSB side) of the value.
-            retval += "00" * (byte_size - len(retval) / 2)
+            retval += "00" * (byte_size - len(retval) // 2)
         return retval
 
     elif endian == 'big':
         retval = ""
         while value != 0:
             retval = "{:02x}".format(value & 0xff) + retval
             value = value >> 8
         if byte_size:
             # Add zero-fill to the left/front (MSB side) of the value.
-            retval = ("00" * (byte_size - len(retval) / 2)) + retval
+            retval = ("00" * (byte_size - len(retval) // 2)) + retval
         return retval
 
     else:
         # pdp is valid but need to add parse code once needed.
         raise Exception("unsupported endian:{}".format(endian))
 
 
 class GdbRemoteEntryBase(object):

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

 
 from __future__ import print_function
 
 
 import re
 import select
 import threading
 import traceback
 import codecs
 
 from six.moves import queue
+from lldbsuite.support import seven
 
 
 def _handle_output_packet_string(packet_contents):
     if (not packet_contents) or (len(packet_contents) < 1):
         return None
     elif packet_contents[0] != "O":
         return None
     elif packet_contents == "OK":
         return None
     else:
-        return packet_contents[1:].decode("hex")
+        return seven.unhexlify(packet_contents[1:])
 
 
 def _dump_queue(the_queue):
     while not the_queue.empty():
         print(codecs.encode(the_queue.get(True), "string_escape"))
         print("\n")
 
 
def _run_method(self):
         if self._logger:
             self._logger.info("socket pump starting")
 
         # Keep looping around until we're asked to stop the thread.
         while not self._stop_thread:
             can_read, _, _ = select.select([self._socket], [], [], 0)
             if can_read and self._socket in can_read:
                 try:
-                    new_bytes = self._socket.recv(4096)
+                    new_bytes = seven.bytes_to_string(self._socket.recv(4096))
                     if self._logger and new_bytes and len(new_bytes) > 0:
                         self._logger.debug(
                             "pump received bytes: {}".format(new_bytes))
                 except:
                     # Likely a closed socket.  Done with the pump thread.
                     if self._logger:
                         self._logger.debug(
                             "socket read failed, stopping pump read thread\n" +