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 <lab...@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 <ztur...@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@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@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
