[Lldb-commits] [lldb] r280207 - Revert r280137 and 280139 and subsequent build fixes

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Wed Aug 31 06:37:53 PDT 2016


    StringExtractor ex("0");
    EXPECT_EQ(0x0, ex.GetHexMaxU64(false, 0)); // instead it crashes



On 31 August 2016 at 14:22, Zachary Turner <zturner at google.com> wrote:
> Can you post the exact contents of the string causing the crash? I'll add a
> unittest for it
>
> On Wed, Aug 31, 2016 at 1:51 AM Pavel Labath via lldb-commits
> <lldb-commits at lists.llvm.org> wrote:
>>
>> Author: labath
>> Date: Wed Aug 31 03:43:37 2016
>> New Revision: 280207
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=280207&view=rev
>> Log:
>> Revert r280137 and 280139 and subsequent build fixes
>>
>> The rewrite of StringExtractor::GetHexMaxU32 changes functionality in a
>> way which makes
>> lldb-server crash. The crash (assert) happens when parsing the
>> "qRegisterInfo0" packet, because
>> the function tries to drop_front more bytes than the packet contains. It's
>> not clear to me
>> whether we should consider this a bug in the caller or the callee, but it
>> any case, it worked
>> before, so I am reverting this until we can figure out what the proper
>> interface should be.
>>
>> Modified:
>>     lldb/trunk/include/lldb/Core/ArchSpec.h
>>     lldb/trunk/include/lldb/Interpreter/Args.h
>>     lldb/trunk/include/lldb/Utility/StringExtractor.h
>>     lldb/trunk/source/Core/ArchSpec.cpp
>>     lldb/trunk/source/Interpreter/Args.cpp
>>     lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
>>
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
>>
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
>>
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
>>     lldb/trunk/source/Utility/StringExtractor.cpp
>>     lldb/trunk/unittests/Utility/StringExtractorTest.cpp
>>
>> Modified: lldb/trunk/include/lldb/Core/ArchSpec.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ArchSpec.h?rev=280207&r1=280206&r2=280207&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Core/ArchSpec.h (original)
>> +++ lldb/trunk/include/lldb/Core/ArchSpec.h Wed Aug 31 03:43:37 2016
>> @@ -265,9 +265,8 @@ public:
>>      ArchSpec (const llvm::Triple &triple);
>>      explicit
>>      ArchSpec (const char *triple_cstr);
>> -    explicit ArchSpec(llvm::StringRef triple_str);
>> -    explicit ArchSpec(const char *triple_cstr, Platform *platform);
>> -    ArchSpec(llvm::StringRef triple_str, Platform *platform);
>> +    explicit
>> +    ArchSpec (const char *triple_cstr, Platform *platform);
>>      //------------------------------------------------------------------
>>      /// Constructor over architecture name.
>>      ///
>>
>> Modified: lldb/trunk/include/lldb/Interpreter/Args.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Args.h?rev=280207&r1=280206&r2=280207&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Interpreter/Args.h (original)
>> +++ lldb/trunk/include/lldb/Interpreter/Args.h Wed Aug 31 03:43:37 2016
>> @@ -202,10 +202,7 @@ public:
>>      ///     The NULL terminated C string of the copy of \a arg_cstr.
>>      //------------------------------------------------------------------
>>      const char *
>> -    AppendArgument(llvm::StringRef arg_str, char quote_char = '\0');
>> -
>> -    const char *
>> -    AppendArgument(const char *arg_cstr, char quote_char = '\0');
>> +    AppendArgument (const char *arg_cstr, char quote_char = '\0');
>>
>>      void
>>      AppendArguments (const Args &rhs);
>> @@ -230,8 +227,6 @@ public:
>>      //------------------------------------------------------------------
>>      const char *
>>      InsertArgumentAtIndex (size_t idx, const char *arg_cstr, char
>> quote_char = '\0');
>> -    const char *
>> -    InsertArgumentAtIndex(size_t idx, llvm::StringRef arg_str, char
>> quote_char = '\0');
>>
>>      //------------------------------------------------------------------
>>      /// Replaces the argument value at index \a idx to \a arg_cstr
>>
>> Modified: lldb/trunk/include/lldb/Utility/StringExtractor.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/StringExtractor.h?rev=280207&r1=280206&r2=280207&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Utility/StringExtractor.h (original)
>> +++ lldb/trunk/include/lldb/Utility/StringExtractor.h Wed Aug 31 03:43:37
>> 2016
>> @@ -112,10 +112,10 @@ public:
>>      char
>>      PeekChar (char fail_value = '\0')
>>      {
>> -        llvm::StringRef str = Peek();
>> -        if (str.empty())
>> -            return fail_value;
>> -        return str[0];
>> +        const char *cstr = Peek();
>> +        if (cstr)
>> +            return cstr[0];
>> +        return fail_value;
>>      }
>>
>>      int
>> @@ -154,6 +154,9 @@ public:
>>      size_t
>>      GetHexBytesAvail (llvm::MutableArrayRef<uint8_t> dest);
>>
>> +    uint64_t
>> +    GetHexWithFixedSize (uint32_t byte_size, bool little_endian, uint64_t
>> fail_value);
>> +
>>      size_t
>>      GetHexByteString (std::string &str);
>>
>> @@ -163,13 +166,13 @@ public:
>>      size_t
>>      GetHexByteStringTerminatedBy (std::string &str,
>>                                    char terminator);
>> -
>> -    llvm::StringRef
>> -    Peek() const
>> +
>> +    const char *
>> +    Peek ()
>>      {
>> -        if (!IsGood())
>> -            return llvm::StringRef();
>> -        return llvm::StringRef(m_packet).drop_front(m_index);
>> +        if (m_index < m_packet.size())
>> +            return m_packet.c_str() + m_index;
>> +        return nullptr;
>>      }
>>
>>  protected:
>>
>> Modified: lldb/trunk/source/Core/ArchSpec.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ArchSpec.cpp?rev=280207&r1=280206&r2=280207&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/source/Core/ArchSpec.cpp (original)
>> +++ lldb/trunk/source/Core/ArchSpec.cpp Wed Aug 31 03:43:37 2016
>> @@ -434,12 +434,6 @@ ArchSpec::ArchSpec (const char *triple_c
>>          SetTriple(triple_cstr, platform);
>>  }
>>
>> -ArchSpec::ArchSpec(llvm::StringRef triple_str, Platform *platform)
>> -    : m_triple(), m_core(kCore_invalid), m_byte_order(eByteOrderInvalid),
>> m_flags(0), m_distribution_id()
>> -{
>> -    if (!triple_str.empty())
>> -        SetTriple(triple_str.str().c_str(), platform);
>> -}
>>
>>  ArchSpec::ArchSpec (const char *triple_cstr) :
>>      m_triple (),
>> @@ -452,13 +446,6 @@ ArchSpec::ArchSpec (const char *triple_c
>>          SetTriple(triple_cstr);
>>  }
>>
>> -ArchSpec::ArchSpec(llvm::StringRef triple_str)
>> -    : m_triple(), m_core(kCore_invalid), m_byte_order(eByteOrderInvalid),
>> m_flags(0), m_distribution_id()
>> -{
>> -    if (!triple_str.empty())
>> -        SetTriple(triple_str.str().c_str());
>> -}
>> -
>>  ArchSpec::ArchSpec(const llvm::Triple &triple) :
>>      m_triple (),
>>      m_core (kCore_invalid),
>>
>> Modified: lldb/trunk/source/Interpreter/Args.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Args.cpp?rev=280207&r1=280206&r2=280207&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/source/Interpreter/Args.cpp (original)
>> +++ lldb/trunk/source/Interpreter/Args.cpp Wed Aug 31 03:43:37 2016
>> @@ -430,19 +430,13 @@ Args::AppendArguments (const char **argv
>>  }
>>
>>  const char *
>> -Args::AppendArgument(llvm::StringRef arg_str, char quote_char)
>> -{
>> -    return InsertArgumentAtIndex(GetArgumentCount(), arg_str,
>> quote_char);
>> -}
>> -
>> -const char *
>> -Args::AppendArgument(const char *arg_cstr, char quote_char)
>> +Args::AppendArgument (const char *arg_cstr, char quote_char)
>>  {
>>      return InsertArgumentAtIndex (GetArgumentCount(), arg_cstr,
>> quote_char);
>>  }
>>
>>  const char *
>> -Args::InsertArgumentAtIndex(size_t idx, llvm::StringRef arg_str, char
>> quote_char)
>> +Args::InsertArgumentAtIndex (size_t idx, const char *arg_cstr, char
>> quote_char)
>>  {
>>      // Since we are using a std::list to hold onto the copied C string
>> and
>>      // we don't have direct access to the elements, we have to iterate to
>> @@ -452,8 +446,8 @@ Args::InsertArgumentAtIndex(size_t idx,
>>      for (pos = m_args.begin(); i > 0 && pos != end; ++pos)
>>          --i;
>>
>> -    pos = m_args.insert(pos, std::string(arg_str.data(),
>> arg_str.size()));
>> -
>> +    pos = m_args.insert(pos, arg_cstr);
>> +
>>      if (idx >= m_args_quote_char.size())
>>      {
>>          m_args_quote_char.resize(idx + 1);
>> @@ -461,19 +455,13 @@ Args::InsertArgumentAtIndex(size_t idx,
>>      }
>>      else
>>          m_args_quote_char.insert(m_args_quote_char.begin() + idx,
>> quote_char);
>> -
>> +
>>      UpdateArgvFromArgs();
>>      return GetArgumentAtIndex(idx);
>>  }
>>
>>  const char *
>> -Args::InsertArgumentAtIndex(size_t idx, const char *arg_cstr, char
>> quote_char)
>> -{
>> -    return InsertArgumentAtIndex(idx, llvm::StringRef(arg_cstr),
>> quote_char);
>> -}
>> -
>> -const char *
>> -Args::ReplaceArgumentAtIndex(size_t idx, const char *arg_cstr, char
>> quote_char)
>> +Args::ReplaceArgumentAtIndex (size_t idx, const char *arg_cstr, char
>> quote_char)
>>  {
>>      // Since we are using a std::list to hold onto the copied C string
>> and
>>      // we don't have direct access to the elements, we have to iterate to
>>
>> Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp?rev=280207&r1=280206&r2=280207&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
>> (original)
>> +++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Wed Aug
>> 31 03:43:37 2016
>> @@ -1620,7 +1620,7 @@ ParseMemoryRegionInfoFromProcMapsLine (c
>>  {
>>      memory_region_info.Clear();
>>
>> -    StringExtractor line_extractor (maps_line);
>> +    StringExtractor line_extractor (maps_line.c_str ());
>>
>>      // Format: {address_start_hex}-{address_end_hex} perms offset  dev
>> inode   pathname
>>      // perms: rwxp   (letter is present if set, '-' if not, final
>> character is p=private, s=shared).
>> @@ -1687,7 +1687,9 @@ ParseMemoryRegionInfoFromProcMapsLine (c
>>      line_extractor.GetU64(0, 10);          // Read the inode number
>>
>>      line_extractor.SkipSpaces();
>> -    memory_region_info.SetName(line_extractor.Peek().str().c_str());
>> +    const char* name = line_extractor.Peek();
>> +    if (name)
>> +        memory_region_info.SetName(name);
>>
>>      return Error ();
>>  }
>>
>> Modified:
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=280207&r1=280206&r2=280207&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
>> (original)
>> +++
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
>> Wed Aug 31 03:43:37 2016
>> @@ -3240,7 +3240,7 @@ GDBRemoteCommunicationClient::ReadFile (
>>          uint32_t retcode = response.GetHexMaxU32(false, UINT32_MAX);
>>          if (retcode == UINT32_MAX)
>>              return retcode;
>> -        const char next = (response.GetBytesLeft() ? response.PeekChar()
>> : 0);
>> +        const char next = (response.Peek() ? *response.Peek() : 0);
>>          if (next == ',')
>>              return 0;
>>          if (next == ';')
>> @@ -3428,7 +3428,7 @@ GDBRemoteCommunicationClient::CalculateM
>>              return false;
>>          if (response.GetChar() != ',')
>>              return false;
>> -        if (response.GetBytesLeft() && response.PeekChar() == 'x')
>> +        if (response.Peek() && *response.Peek() == 'x')
>>              return false;
>>          low = response.GetHexMaxU64(false, UINT64_MAX);
>>          high = response.GetHexMaxU64(false, UINT64_MAX);
>>
>> Modified:
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp?rev=280207&r1=280206&r2=280207&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
>> (original)
>> +++
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
>> Wed Aug 31 03:43:37 2016
>> @@ -966,7 +966,7 @@ GDBRemoteCommunicationServerCommon::Hand
>>      {
>>          std::string str;
>>          packet.GetHexByteString(str);
>> -
>> m_process_launch_info.GetEnvironmentEntries().AppendArgument(str);
>> +
>> m_process_launch_info.GetEnvironmentEntries().AppendArgument(str.c_str());
>>          return SendOKResponse();
>>      }
>>      return SendErrorResponse(12);
>> @@ -979,7 +979,8 @@ GDBRemoteCommunicationServerCommon::Hand
>>      const uint32_t bytes_left = packet.GetBytesLeft();
>>      if (bytes_left > 0)
>>      {
>> -        ArchSpec arch_spec(packet.Peek(), nullptr);
>> +        const char* arch_triple = packet.Peek();
>> +        ArchSpec arch_spec(arch_triple,NULL);
>>          m_process_launch_info.SetArchitecture(arch_spec);
>>          return SendOKResponse();
>>      }
>>
>> Modified:
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=280207&r1=280206&r2=280207&view=diff
>>
>> ==============================================================================
>> ---
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
>> (original)
>> +++
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
>> Wed Aug 31 03:43:37 2016
>> @@ -1186,7 +1186,7 @@ GDBRemoteCommunicationServerLLGS::Handle
>>      if (packet.GetBytesLeft () > 0)
>>      {
>>          // FIXME add continue at address support for
>> $C{signo}[;{continue-address}].
>> -        if (packet.PeekChar() == ';')
>> +        if (*packet.Peek () == ';')
>>              return SendUnimplementedResponse
>> (packet.GetStringRef().c_str());
>>          else
>>              return SendIllFormedResponse (packet, "unexpected content
>> after $C{signal-number}");
>> @@ -1257,8 +1257,7 @@ GDBRemoteCommunicationServerLLGS::Handle
>>      if (has_continue_address)
>>      {
>>          if (log)
>> -            log->Printf("GDBRemoteCommunicationServerLLGS::%s not
>> implemented for c{address} variant [%s remains]",
>> -                        __FUNCTION__, packet.Peek().str().c_str());
>> +            log->Printf ("GDBRemoteCommunicationServerLLGS::%s not
>> implemented for c{address} variant [%s remains]", __FUNCTION__, packet.Peek
>> ());
>>          return SendUnimplementedResponse (packet.GetStringRef().c_str());
>>      }
>>
>> @@ -1319,13 +1318,13 @@ GDBRemoteCommunicationServerLLGS::Handle
>>      }
>>
>>      // Check if this is all continue (no options or ";c").
>> -    if (packet.Peek() == ";c")
>> +    if (::strcmp (packet.Peek (), ";c") == 0)
>>      {
>>          // Move past the ';', then do a simple 'c'.
>>          packet.SetFilePos (packet.GetFilePos () + 1);
>>          return Handle_c (packet);
>>      }
>> -    else if (packet.Peek() == ";s")
>> +    else if (::strcmp (packet.Peek (), ";s") == 0)
>>      {
>>          // Move past the ';', then do a simple 's'.
>>          packet.SetFilePos (packet.GetFilePos () + 1);
>> @@ -1342,7 +1341,7 @@ GDBRemoteCommunicationServerLLGS::Handle
>>
>>      ResumeActionList thread_actions;
>>
>> -    while (packet.GetBytesLeft() && packet.PeekChar() == ';')
>> +    while (packet.GetBytesLeft () && *packet.Peek () == ';')
>>      {
>>          // Skip the semi-colon.
>>          packet.GetChar ();
>> @@ -1384,7 +1383,7 @@ GDBRemoteCommunicationServerLLGS::Handle
>>          }
>>
>>          // Parse out optional :{thread-id} value.
>> -        if (packet.GetBytesLeft() && packet.PeekChar() == ':')
>> +        if (packet.GetBytesLeft () && (*packet.Peek () == ':'))
>>          {
>>              // Consume the separator.
>>              packet.GetChar ();
>> @@ -2927,7 +2926,7 @@ GDBRemoteCommunicationServerLLGS::GetThr
>>          return thread_sp;
>>
>>      // Parse out thread: portion.
>> -    if (packet.Peek().startswith("thread:"))
>> +    if (strncmp (packet.Peek (), "thread:", strlen("thread:")) != 0)
>>      {
>>          if (log)
>>              log->Printf ("GDBRemoteCommunicationServerLLGS::%s gdb-remote
>> parse error: expected 'thread:' but not found, packet contents = '%s'",
>> __FUNCTION__, packet.GetStringRef ().c_str ());
>>
>> Modified: lldb/trunk/source/Utility/StringExtractor.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/StringExtractor.cpp?rev=280207&r1=280206&r2=280207&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/source/Utility/StringExtractor.cpp (original)
>> +++ lldb/trunk/source/Utility/StringExtractor.cpp Wed Aug 31 03:43:37 2016
>> @@ -16,8 +16,6 @@
>>  #include <tuple>
>>  // Other libraries and framework includes
>>  // Project includes
>> -#include "llvm/ADT/Optional.h"
>> -#include "llvm/Support/Endian.h"
>>
>>  static inline int
>>  xdigit_to_sint (char ch)
>> @@ -100,16 +98,6 @@ StringExtractor::GetChar (char fail_valu
>>      return fail_value;
>>  }
>>
>> -static llvm::Optional<uint8_t>
>> -translateHexChar(char ch1, char ch2)
>> -{
>> -    const int hi_nibble = xdigit_to_sint(ch1);
>> -    const int lo_nibble = xdigit_to_sint(ch2);
>> -    if (hi_nibble == -1 || lo_nibble == -1)
>> -        return llvm::None;
>> -    return (uint8_t)((hi_nibble << 4) + lo_nibble);
>> -}
>> -
>>  //----------------------------------------------------------------------
>>  // If a pair of valid hex digits exist at the head of the
>>  // StringExtractor they are decoded into an unsigned byte and returned
>> @@ -123,12 +111,17 @@ StringExtractor::DecodeHexU8()
>>  {
>>      SkipSpaces();
>>      if (GetBytesLeft() < 2)
>> +    {
>>          return -1;
>> -    auto result = translateHexChar(m_packet[m_index], m_packet[m_index +
>> 1]);
>> -    if (!result.hasValue())
>> +    }
>> +    const int hi_nibble = xdigit_to_sint(m_packet[m_index]);
>> +    const int lo_nibble = xdigit_to_sint(m_packet[m_index+1]);
>> +    if (hi_nibble == -1 || lo_nibble == -1)
>> +    {
>>          return -1;
>> +    }
>>      m_index += 2;
>> -    return *result;
>> +    return (uint8_t)((hi_nibble << 4) + lo_nibble);
>>  }
>>
>>  //----------------------------------------------------------------------
>> @@ -236,60 +229,131 @@ StringExtractor::GetS64 (int64_t fail_va
>>      return fail_value;
>>  }
>>
>> +
>>  uint32_t
>>  StringExtractor::GetHexMaxU32 (bool little_endian, uint32_t fail_value)
>>  {
>> -    SkipSpaces();
>> +    uint32_t result = 0;
>> +    uint32_t nibble_count = 0;
>>
>> -    // Allocate enough space for 2 uint32's.  In big endian, if the user
>> writes
>> -    // "AB" then this should be treated as 0xAB, not 0xAB000000.  In
>> order to
>> -    // do this, we decode into the second half of the array, and then
>> shift the
>> -    // starting point of the big endian translation left by however many
>> bytes
>> -    // of a uint32 were missing from the input.  We're essentially
>> padding left
>> -    // with 0's.
>> -    uint8_t bytes[2 * sizeof(uint32_t) - 1] = {0};
>> -    llvm::MutableArrayRef<uint8_t> byte_array(bytes);
>> -    llvm::MutableArrayRef<uint8_t> decode_loc =
>> byte_array.take_back(sizeof(uint32_t));
>> -    uint32_t bytes_decoded = GetHexBytesAvail(decode_loc);
>> -    if (bytes_decoded == sizeof(uint32_t) && ::isxdigit(PeekChar()))
>> -        return fail();
>> -
>> -    using namespace llvm::support;
>> +    SkipSpaces();
>>      if (little_endian)
>> -        return endian::read<uint32_t,
>> endianness::little>(decode_loc.data());
>> +    {
>> +        uint32_t shift_amount = 0;
>> +        while (m_index < m_packet.size() && ::isxdigit
>> (m_packet[m_index]))
>> +        {
>> +            // Make sure we don't exceed the size of a uint32_t...
>> +            if (nibble_count >= (sizeof(uint32_t) * 2))
>> +            {
>> +                m_index = UINT64_MAX;
>> +                return fail_value;
>> +            }
>> +
>> +            uint8_t nibble_lo;
>> +            uint8_t nibble_hi = xdigit_to_sint (m_packet[m_index]);
>> +            ++m_index;
>> +            if (m_index < m_packet.size() && ::isxdigit
>> (m_packet[m_index]))
>> +            {
>> +                nibble_lo = xdigit_to_sint (m_packet[m_index]);
>> +                ++m_index;
>> +                result |= ((uint32_t)nibble_hi << (shift_amount + 4));
>> +                result |= ((uint32_t)nibble_lo << shift_amount);
>> +                nibble_count += 2;
>> +                shift_amount += 8;
>> +            }
>> +            else
>> +            {
>> +                result |= ((uint32_t)nibble_hi << shift_amount);
>> +                nibble_count += 1;
>> +                shift_amount += 4;
>> +            }
>> +
>> +        }
>> +    }
>>      else
>>      {
>> -        decode_loc = byte_array.drop_front(bytes_decoded -
>> 1).take_front(sizeof(uint32_t));
>> -        return endian::read<uint32_t,
>> endianness::big>(decode_loc.data());
>> +        while (m_index < m_packet.size() && ::isxdigit
>> (m_packet[m_index]))
>> +        {
>> +            // Make sure we don't exceed the size of a uint32_t...
>> +            if (nibble_count >= (sizeof(uint32_t) * 2))
>> +            {
>> +                m_index = UINT64_MAX;
>> +                return fail_value;
>> +            }
>> +
>> +            uint8_t nibble = xdigit_to_sint (m_packet[m_index]);
>> +            // Big Endian
>> +            result <<= 4;
>> +            result |= nibble;
>> +
>> +            ++m_index;
>> +            ++nibble_count;
>> +        }
>>      }
>> +    return result;
>>  }
>>
>>  uint64_t
>>  StringExtractor::GetHexMaxU64 (bool little_endian, uint64_t fail_value)
>>  {
>> -    SkipSpaces();
>> -
>> -    // Allocate enough space for 2 uint64's.  In big endian, if the user
>> writes
>> -    // "AB" then this should be treated as 0x000000AB, not 0xAB000000.
>> In order
>> -    // to do this, we decode into the second half of the array, and then
>> shift
>> -    // the starting point of the big endian translation left by however
>> many bytes
>> -    // of a uint32 were missing from the input.  We're essentially
>> padding left
>> -    // with 0's.
>> -    uint8_t bytes[2 * sizeof(uint64_t) - 1] = {0};
>> -    llvm::MutableArrayRef<uint8_t> byte_array(bytes);
>> -    llvm::MutableArrayRef<uint8_t> decode_loc =
>> byte_array.take_back(sizeof(uint64_t));
>> -    uint32_t bytes_decoded = GetHexBytesAvail(decode_loc);
>> -    if (bytes_decoded == sizeof(uint64_t) && ::isxdigit(PeekChar()))
>> -        return fail();
>> +    uint64_t result = 0;
>> +    uint32_t nibble_count = 0;
>>
>> -    using namespace llvm::support;
>> +    SkipSpaces();
>>      if (little_endian)
>> -        return endian::read<uint64_t,
>> endianness::little>(decode_loc.data());
>> +    {
>> +        uint32_t shift_amount = 0;
>> +        while (m_index < m_packet.size() && ::isxdigit
>> (m_packet[m_index]))
>> +        {
>> +            // Make sure we don't exceed the size of a uint64_t...
>> +            if (nibble_count >= (sizeof(uint64_t) * 2))
>> +            {
>> +                m_index = UINT64_MAX;
>> +                return fail_value;
>> +            }
>> +
>> +            uint8_t nibble_lo;
>> +            uint8_t nibble_hi = xdigit_to_sint (m_packet[m_index]);
>> +            ++m_index;
>> +            if (m_index < m_packet.size() && ::isxdigit
>> (m_packet[m_index]))
>> +            {
>> +                nibble_lo = xdigit_to_sint (m_packet[m_index]);
>> +                ++m_index;
>> +                result |= ((uint64_t)nibble_hi << (shift_amount + 4));
>> +                result |= ((uint64_t)nibble_lo << shift_amount);
>> +                nibble_count += 2;
>> +                shift_amount += 8;
>> +            }
>> +            else
>> +            {
>> +                result |= ((uint64_t)nibble_hi << shift_amount);
>> +                nibble_count += 1;
>> +                shift_amount += 4;
>> +            }
>> +
>> +        }
>> +    }
>>      else
>>      {
>> -        decode_loc = byte_array.drop_front(bytes_decoded -
>> 1).take_front(sizeof(uint64_t));
>> -        return endian::read<uint64_t,
>> endianness::big>(decode_loc.data());
>> +        while (m_index < m_packet.size() && ::isxdigit
>> (m_packet[m_index]))
>> +        {
>> +            // Make sure we don't exceed the size of a uint64_t...
>> +            if (nibble_count >= (sizeof(uint64_t) * 2))
>> +            {
>> +                m_index = UINT64_MAX;
>> +                return fail_value;
>> +            }
>> +
>> +            uint8_t nibble = xdigit_to_sint (m_packet[m_index]);
>> +            // Big Endian
>> +            result <<= 4;
>> +            result |= nibble;
>> +
>> +            ++m_index;
>> +            ++nibble_count;
>> +        }
>>      }
>> +    return result;
>>  }
>>
>>  size_t
>> @@ -333,6 +397,41 @@ StringExtractor::GetHexBytesAvail (llvm:
>>      return bytes_extracted;
>>  }
>>
>> +// Consume ASCII hex nibble character pairs until we have decoded
>> byte_size
>> +// bytes of data.
>> +
>> +uint64_t
>> +StringExtractor::GetHexWithFixedSize (uint32_t byte_size, bool
>> little_endian, uint64_t fail_value)
>> +{
>> +    if (byte_size <= 8 && GetBytesLeft() >= byte_size * 2)
>> +    {
>> +        uint64_t result = 0;
>> +        uint32_t i;
>> +        if (little_endian)
>> +        {
>> +            // Little Endian
>> +            uint32_t shift_amount;
>> +            for (i = 0, shift_amount = 0;
>> +                 i < byte_size && IsGood();
>> +                 ++i, shift_amount += 8)
>> +            {
>> +                result |= ((uint64_t)GetHexU8() << shift_amount);
>> +            }
>> +        }
>> +        else
>> +        {
>> +            // Big Endian
>> +            for (i = 0; i < byte_size && IsGood(); ++i)
>> +            {
>> +                result <<= 8;
>> +                result |= GetHexU8();
>> +            }
>> +        }
>> +    }
>> +    m_index = UINT64_MAX;
>> +    return fail_value;
>> +}
>> +
>>  size_t
>>  StringExtractor::GetHexByteString (std::string &str)
>>  {
>> @@ -348,16 +447,11 @@ size_t
>>  StringExtractor::GetHexByteStringFixedLength (std::string &str, uint32_t
>> nibble_length)
>>  {
>>      str.clear();
>> -    llvm::StringRef nibs = Peek().take_front(nibble_length);
>> -    while (nibs.size() >= 2)
>> -    {
>> -        auto ch = translateHexChar(nibs[0], nibs[1]);
>> -        if (!ch.hasValue())
>> -            break;
>> -        str.push_back(*ch);
>> -        nibs = nibs.drop_front(2);
>> -    }
>> -    m_index += str.size() * 2;
>> +
>> +    uint32_t nibble_count = 0;
>> +    for (const char *pch = Peek(); (nibble_count < nibble_length) && (pch
>> != nullptr); str.append(1, GetHexU8(0, false)), pch = Peek (), nibble_count
>> += 2)
>> +    {}
>> +
>>      return str.size();
>>  }
>>
>> @@ -369,7 +463,7 @@ StringExtractor::GetHexByteStringTermina
>>      char ch;
>>      while ((ch = GetHexU8(0,false)) != '\0')
>>          str.append(1, ch);
>> -    if (GetBytesLeft() > 0 && PeekChar() == terminator)
>> +    if (Peek() && *Peek() == terminator)
>>          return str.size();
>>
>>      str.clear();
>>
>> Modified: lldb/trunk/unittests/Utility/StringExtractorTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/StringExtractorTest.cpp?rev=280207&r1=280206&r2=280207&view=diff
>>
>> ==============================================================================
>> --- lldb/trunk/unittests/Utility/StringExtractorTest.cpp (original)
>> +++ lldb/trunk/unittests/Utility/StringExtractorTest.cpp Wed Aug 31
>> 03:43:37 2016
>> @@ -20,6 +20,7 @@ TEST_F (StringExtractorTest, InitEmpty)
>>      ASSERT_STREQ (kEmptyString, ex.GetStringRef().c_str());
>>      ASSERT_EQ (true, ex.Empty());
>>      ASSERT_EQ (0u, ex.GetBytesLeft());
>> +    ASSERT_EQ (nullptr, ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, InitMisc)
>> @@ -32,7 +33,7 @@ TEST_F (StringExtractorTest, InitMisc)
>>      ASSERT_STREQ (kInitMiscString, ex.GetStringRef().c_str());
>>      ASSERT_EQ (false, ex.Empty());
>>      ASSERT_EQ (sizeof(kInitMiscString)-1, ex.GetBytesLeft());
>> -    ASSERT_EQ(kInitMiscString[0], ex.PeekChar());
>> +    ASSERT_EQ (kInitMiscString[0], *ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, DecodeHexU8_Underflow)
>> @@ -45,6 +46,7 @@ TEST_F (StringExtractorTest, DecodeHexU8
>>      ASSERT_EQ (0u, ex.GetFilePos());
>>      ASSERT_EQ (true, ex.Empty());
>>      ASSERT_EQ (0u, ex.GetBytesLeft());
>> +    ASSERT_EQ (nullptr, ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, DecodeHexU8_Underflow2)
>> @@ -56,7 +58,7 @@ TEST_F (StringExtractorTest, DecodeHexU8
>>      ASSERT_EQ (true, ex.IsGood());
>>      ASSERT_EQ (0u, ex.GetFilePos());
>>      ASSERT_EQ (1u, ex.GetBytesLeft());
>> -    ASSERT_EQ('1', ex.PeekChar());
>> +    ASSERT_EQ ('1', *ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, DecodeHexU8_InvalidHex)
>> @@ -68,7 +70,7 @@ TEST_F (StringExtractorTest, DecodeHexU8
>>      ASSERT_EQ (true, ex.IsGood());
>>      ASSERT_EQ (0u, ex.GetFilePos());
>>      ASSERT_EQ (2u, ex.GetBytesLeft());
>> -    ASSERT_EQ('x', ex.PeekChar());
>> +    ASSERT_EQ ('x', *ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, DecodeHexU8_InvalidHex2)
>> @@ -80,7 +82,7 @@ TEST_F (StringExtractorTest, DecodeHexU8
>>      ASSERT_EQ (true, ex.IsGood());
>>      ASSERT_EQ (0u, ex.GetFilePos());
>>      ASSERT_EQ (2u, ex.GetBytesLeft());
>> -    ASSERT_EQ('a', ex.PeekChar());
>> +    ASSERT_EQ ('a', *ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, DecodeHexU8_Exact)
>> @@ -92,6 +94,7 @@ TEST_F (StringExtractorTest, DecodeHexU8
>>      ASSERT_EQ (true, ex.IsGood());
>>      ASSERT_EQ (2u, ex.GetFilePos());
>>      ASSERT_EQ (0u, ex.GetBytesLeft());
>> +    ASSERT_EQ (nullptr, ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, DecodeHexU8_Extra)
>> @@ -103,7 +106,7 @@ TEST_F (StringExtractorTest, DecodeHexU8
>>      ASSERT_EQ (true, ex.IsGood());
>>      ASSERT_EQ (2u, ex.GetFilePos());
>>      ASSERT_EQ (2u, ex.GetBytesLeft());
>> -    ASSERT_EQ('3', ex.PeekChar());
>> +    ASSERT_EQ ('3', *ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexU8_Underflow)
>> @@ -116,6 +119,7 @@ TEST_F (StringExtractorTest, GetHexU8_Un
>>      ASSERT_EQ (UINT64_MAX, ex.GetFilePos());
>>      ASSERT_EQ (true, ex.Empty());
>>      ASSERT_EQ (0u, ex.GetBytesLeft());
>> +    ASSERT_EQ (nullptr, ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexU8_Underflow2)
>> @@ -127,6 +131,7 @@ TEST_F (StringExtractorTest, GetHexU8_Un
>>      ASSERT_EQ (false, ex.IsGood());
>>      ASSERT_EQ (UINT64_MAX, ex.GetFilePos());
>>      ASSERT_EQ (0u, ex.GetBytesLeft());
>> +    ASSERT_EQ (nullptr, ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexU8_InvalidHex)
>> @@ -138,6 +143,7 @@ TEST_F (StringExtractorTest, GetHexU8_In
>>      ASSERT_EQ (false, ex.IsGood());
>>      ASSERT_EQ (UINT64_MAX, ex.GetFilePos());
>>      ASSERT_EQ (0u, ex.GetBytesLeft());
>> +    ASSERT_EQ (nullptr, ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexU8_Exact)
>> @@ -149,6 +155,7 @@ TEST_F (StringExtractorTest, GetHexU8_Ex
>>      ASSERT_EQ (true, ex.IsGood());
>>      ASSERT_EQ (2u, ex.GetFilePos());
>>      ASSERT_EQ (0u, ex.GetBytesLeft());
>> +    ASSERT_EQ (nullptr, ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexU8_Extra)
>> @@ -160,7 +167,7 @@ TEST_F (StringExtractorTest, GetHexU8_Ex
>>      ASSERT_EQ (true, ex.IsGood());
>>      ASSERT_EQ (2u, ex.GetFilePos());
>>      ASSERT_EQ (2u, ex.GetBytesLeft());
>> -    ASSERT_EQ('3', ex.PeekChar());
>> +    ASSERT_EQ ('3', *ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexU8_Underflow_NoEof)
>> @@ -174,6 +181,7 @@ TEST_F (StringExtractorTest, GetHexU8_Un
>>      ASSERT_EQ (UINT64_MAX, ex.GetFilePos());
>>      ASSERT_EQ (true, ex.Empty());
>>      ASSERT_EQ (0u, ex.GetBytesLeft());
>> +    ASSERT_EQ (nullptr, ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexU8_Underflow2_NoEof)
>> @@ -186,7 +194,7 @@ TEST_F (StringExtractorTest, GetHexU8_Un
>>      ASSERT_EQ (true, ex.IsGood());
>>      ASSERT_EQ (0u, ex.GetFilePos());
>>      ASSERT_EQ (1u, ex.GetBytesLeft());
>> -    ASSERT_EQ('1', ex.PeekChar());
>> +    ASSERT_EQ ('1', *ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexU8_InvalidHex_NoEof)
>> @@ -199,7 +207,7 @@ TEST_F (StringExtractorTest, GetHexU8_In
>>      ASSERT_EQ (true, ex.IsGood());
>>      ASSERT_EQ (0u, ex.GetFilePos());
>>      ASSERT_EQ (2u, ex.GetBytesLeft());
>> -    ASSERT_EQ('x', ex.PeekChar());
>> +    ASSERT_EQ ('x', *ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexU8_Exact_NoEof)
>> @@ -212,6 +220,7 @@ TEST_F (StringExtractorTest, GetHexU8_Ex
>>      ASSERT_EQ (true, ex.IsGood());
>>      ASSERT_EQ (2u, ex.GetFilePos());
>>      ASSERT_EQ (0u, ex.GetBytesLeft());
>> +    ASSERT_EQ (nullptr, ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexU8_Extra_NoEof)
>> @@ -224,7 +233,7 @@ TEST_F (StringExtractorTest, GetHexU8_Ex
>>      ASSERT_EQ (true, ex.IsGood());
>>      ASSERT_EQ (2u, ex.GetFilePos());
>>      ASSERT_EQ (2u, ex.GetBytesLeft());
>> -    ASSERT_EQ('3', ex.PeekChar());
>> +    ASSERT_EQ ('3', *ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexBytes)
>> @@ -248,7 +257,7 @@ TEST_F (StringExtractorTest, GetHexBytes
>>      ASSERT_EQ(2*kValidHexPairs, ex.GetFilePos());
>>      ASSERT_EQ(false, ex.Empty());
>>      ASSERT_EQ(4u, ex.GetBytesLeft());
>> -    ASSERT_EQ('x', ex.PeekChar());
>> +    ASSERT_EQ('x', *ex.Peek());
>>  }
>>
>>  TEST_F(StringExtractorTest, GetHexBytes_FullString)
>> @@ -335,6 +344,7 @@ TEST_F (StringExtractorTest, GetHexBytes
>>      ASSERT_EQ(UINT64_MAX, ex.GetFilePos());
>>      ASSERT_EQ(false, ex.Empty());
>>      ASSERT_EQ(0u, ex.GetBytesLeft());
>> +    ASSERT_EQ(0, ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexBytes_Partial)
>> @@ -364,7 +374,7 @@ TEST_F (StringExtractorTest, GetHexBytes
>>      ASSERT_EQ(kReadBytes*2, ex.GetFilePos());
>>      ASSERT_EQ(false, ex.Empty());
>>      ASSERT_EQ(12u, ex.GetBytesLeft());
>> -    ASSERT_EQ('2', ex.PeekChar());
>> +    ASSERT_EQ('2', *ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexBytesAvail)
>> @@ -388,7 +398,7 @@ TEST_F (StringExtractorTest, GetHexBytes
>>      ASSERT_EQ(2*kValidHexPairs, ex.GetFilePos());
>>      ASSERT_EQ(false, ex.Empty());
>>      ASSERT_EQ(4u, ex.GetBytesLeft());
>> -    ASSERT_EQ('x', ex.PeekChar());
>> +    ASSERT_EQ('x', *ex.Peek());
>>  }
>>
>>  TEST_F(StringExtractorTest, GetHexBytesAvail_FullString)
>> @@ -471,7 +481,7 @@ TEST_F (StringExtractorTest, GetHexBytes
>>      ASSERT_EQ(kValidHexPairs*2, ex.GetFilePos());
>>      ASSERT_EQ(false, ex.Empty());
>>      ASSERT_EQ(4u, ex.GetBytesLeft());
>> -    ASSERT_EQ('x', ex.PeekChar());
>> +    ASSERT_EQ('x', *ex.Peek());
>>  }
>>
>>  TEST_F (StringExtractorTest, GetHexBytesAvail_Partial)
>> @@ -501,7 +511,7 @@ TEST_F (StringExtractorTest, GetHexBytes
>>      ASSERT_EQ(kReadBytes*2, ex.GetFilePos());
>>      ASSERT_EQ(false, ex.Empty());
>>      ASSERT_EQ(12u, ex.GetBytesLeft());
>> -    ASSERT_EQ('2', ex.PeekChar());
>> +    ASSERT_EQ('2', *ex.Peek());
>>  }
>>
>>  TEST_F(StringExtractorTest, GetNameColonValueSuccess)
>>
>>
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


More information about the lldb-commits mailing list