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

Zachary Turner via lldb-commits lldb-commits at lists.llvm.org
Wed Aug 31 06:45:13 PDT 2016


Thanks, I'll check it out. Are there any plans to make buildbots send
emails? It seems less useful when we can't tell when we're breaking
something
On Wed, Aug 31, 2016 at 6:38 AM Pavel Labath <labath at google.com> wrote:

>     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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160831/4bba303e/attachment-0001.html>


More information about the lldb-commits mailing list