[Lldb-commits] [lldb] r180141 - The new ReadStringFromMemory() API does not work correctly with NSStrings that have an explicit length and no NULL terminator
Enrico Granata
egranata at apple.com
Tue Apr 23 14:39:21 PDT 2013
Looks like it was a u vs. U issue - should be fixed now
Enrico Granata
✉ egranata@.com
✆ 27683
On Apr 23, 2013, at 1:37 PM, "Malea, Daniel" <daniel.malea at intel.com> wrote:
> Enrico,
>
> This commit introduces a regression in TestChar1632T.py:
>
> See http://lab.llvm.org:8011/builders/lldb-x86_64-debian-clang/builds/2530
>
> FAIL: LLDB (clang-x86_64) :: test_with_dwarf
> (TestChar1632T.Char1632TestCase)
> ======================================================================
> FAIL: test_with_dwarf (TestChar1632T.Char1632TestCase)
> Test that the C++11 support for char16_t and char32_t works correctly.
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File
> "/home/llvmbb/llvm-build-dir/lldb-x86_64-clang/llvm/tools/lldb/test/lldbtes
> t.py", line 365, in wrapper
> return func(self, *args, **kwargs)
> File
> "/home/llvmbb/llvm-build-dir/lldb-x86_64-clang/llvm/tools/lldb/test/lang/cp
> p/char1632_t/TestChar1632T.py", line 27, in test_with_dwarf
> self.char1632()
> File
> "/home/llvmbb/llvm-build-dir/lldb-x86_64-clang/llvm/tools/lldb/test/lang/cp
> p/char1632_t/TestChar1632T.py", line 58, in char1632
> substrs = ['(const char16_t *) cs16 = ','(const char32_t *) cs32 =
> ','u"hello world ྒྙྐ"','U"hello world ྒྙྐ"'])
> File
> "/home/llvmbb/llvm-build-dir/lldb-x86_64-clang/llvm/tools/lldb/test/lldbtes
> t.py", line 1514, in expect
> msg if msg else EXP_MSG(str, exe))
> AssertionError: False is not True : 'U"hello world ྒྙྐ"' returns expected
> result
>
>
>
>
> Dan
>
>
>
> On 2013-04-23 4:05 PM, "Enrico Granata" <egranata at apple.com> wrote:
>
>> Author: enrico
>> Date: Tue Apr 23 15:05:05 2013
>> New Revision: 180141
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=180141&view=rev
>> Log:
>> The new ReadStringFromMemory() API does not work correctly with NSStrings
>> that have an explicit length and no NULL terminator
>> This checkin reverts NSString to the old behavior when appropriate, and
>> cleans up the syntax to call the UTF Reader&Dumper function
>> Incidentally, add a "-d" command-line flag to redo.py with the same
>> semantics as "-d" in dotest.py
>>
>>
>> Modified:
>> lldb/trunk/source/DataFormatters/CXXFormatterFunctions.cpp
>> lldb/trunk/test/redo.py
>>
>> Modified: lldb/trunk/source/DataFormatters/CXXFormatterFunctions.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/DataFormatters/CXXFo
>> rmatterFunctions.cpp?rev=180141&r1=180140&r2=180141&view=diff
>> ==========================================================================
>> ====
>> --- lldb/trunk/source/DataFormatters/CXXFormatterFunctions.cpp (original)
>> +++ lldb/trunk/source/DataFormatters/CXXFormatterFunctions.cpp Tue Apr 23
>> 15:05:05 2013
>> @@ -216,9 +216,9 @@ DumpUTFBufferToStream (ConversionResult
>>
>> if (ConvertFunction)
>> {
>> - utf8_data_buffer_sp.reset(new
>> DataBufferHeap(bufferSPSize,0));
>> + utf8_data_buffer_sp.reset(new
>> DataBufferHeap(4*bufferSPSize,0));
>> utf8_data_ptr = (UTF8*)utf8_data_buffer_sp->GetBytes();
>> - utf8_data_end_ptr = utf8_data_ptr + bufferSPSize;
>> + utf8_data_end_ptr = utf8_data_ptr +
>> utf8_data_buffer_sp->GetByteSize();
>> ConvertFunction ( (const SourceDataType**)&data_ptr,
>> data_end_ptr, &utf8_data_ptr, utf8_data_end_ptr, lenientConversion );
>> utf8_data_ptr = (UTF8*)utf8_data_buffer_sp->GetBytes(); //
>> needed because the ConvertFunction will change the value of the data_ptr
>> }
>> @@ -246,21 +246,151 @@ DumpUTFBufferToStream (ConversionResult
>> }
>>
>> template<typename SourceDataType>
>> +class ReadUTFBufferAndDumpToStreamOptions
>> +{
>> +public:
>> + typedef ConversionResult (*ConvertFunctionType) (const
>> SourceDataType**,
>> + const
>> SourceDataType*,
>> + UTF8**,
>> + UTF8*,
>> + ConversionFlags);
>> +
>> + ReadUTFBufferAndDumpToStreamOptions () :
>> + m_conversion_function(NULL),
>> + m_location(0),
>> + m_process_sp(),
>> + m_stream(NULL),
>> + m_prefix_token('@'),
>> + m_quote('"'),
>> + m_source_size(0),
>> + m_needs_zero_termination(true)
>> + {
>> + }
>> +
>> + ReadUTFBufferAndDumpToStreamOptions&
>> + SetConversionFunction (ConvertFunctionType f)
>> + {
>> + m_conversion_function = f;
>> + return *this;
>> + }
>> +
>> + ConvertFunctionType
>> + GetConversionFunction () const
>> + {
>> + return m_conversion_function;
>> + }
>> +
>> + ReadUTFBufferAndDumpToStreamOptions&
>> + SetLocation (uint64_t l)
>> + {
>> + m_location = l;
>> + return *this;
>> + }
>> +
>> + uint64_t
>> + GetLocation () const
>> + {
>> + return m_location;
>> + }
>> +
>> + ReadUTFBufferAndDumpToStreamOptions&
>> + SetProcessSP (ProcessSP p)
>> + {
>> + m_process_sp = p;
>> + return *this;
>> + }
>> +
>> + ProcessSP
>> + GetProcessSP () const
>> + {
>> + return m_process_sp;
>> + }
>> +
>> + ReadUTFBufferAndDumpToStreamOptions&
>> + SetStream (Stream* s)
>> + {
>> + m_stream = s;
>> + return *this;
>> + }
>> +
>> + Stream*
>> + GetStream () const
>> + {
>> + return m_stream;
>> + }
>> +
>> + ReadUTFBufferAndDumpToStreamOptions&
>> + SetPrefixToken (char p)
>> + {
>> + m_prefix_token = p;
>> + return *this;
>> + }
>> +
>> + char
>> + GetPrefixToken () const
>> + {
>> + return m_prefix_token;
>> + }
>> +
>> + ReadUTFBufferAndDumpToStreamOptions&
>> + SetQuote (char q)
>> + {
>> + m_quote = q;
>> + return *this;
>> + }
>> +
>> + char
>> + GetQuote () const
>> + {
>> + return m_quote;
>> + }
>> +
>> + ReadUTFBufferAndDumpToStreamOptions&
>> + SetSourceSize (uint32_t s)
>> + {
>> + m_source_size = s;
>> + return *this;
>> + }
>> +
>> + uint32_t
>> + GetSourceSize () const
>> + {
>> + return m_source_size;
>> + }
>> +
>> + ReadUTFBufferAndDumpToStreamOptions&
>> + SetNeedsZeroTermination (bool z)
>> + {
>> + m_needs_zero_termination = z;
>> + return *this;
>> + }
>> +
>> + bool
>> + GetNeedsZeroTermination () const
>> + {
>> + return m_needs_zero_termination;
>> + }
>> +
>> +private:
>> + ConvertFunctionType m_conversion_function;
>> + uint64_t m_location;
>> + ProcessSP m_process_sp;
>> + Stream* m_stream;
>> + char m_prefix_token;
>> + char m_quote;
>> + uint32_t m_source_size;
>> + bool m_needs_zero_termination;
>> +};
>> +
>> +template<typename SourceDataType>
>> static bool
>> -ReadUTFBufferAndDumpToStream (ConversionResult (*ConvertFunction) (const
>> SourceDataType**,
>> - const
>> SourceDataType*,
>> -
>> UTF8**,
>> - UTF8*,
>> -
>> ConversionFlags),
>> - uint64_t location,
>> - const ProcessSP& process_sp,
>> - Stream& stream,
>> - char prefix_token = '@',
>> - char quote = '"',
>> - uint32_t sourceSize = 0)
>> +ReadUTFBufferAndDumpToStream (const
>> ReadUTFBufferAndDumpToStreamOptions<SourceDataType>& options)
>> {
>> - if (location == 0 || location == LLDB_INVALID_ADDRESS)
>> + if (options.GetLocation() == 0 || options.GetLocation() ==
>> LLDB_INVALID_ADDRESS)
>> return false;
>> +
>> + ProcessSP process_sp(options.GetProcessSP());
>> +
>> if (!process_sp)
>> return false;
>>
>> @@ -269,11 +399,20 @@ ReadUTFBufferAndDumpToStream (Conversion
>> if (origin_encoding != 8 && origin_encoding != 16 && origin_encoding
>> != 32)
>> return false;
>> // if not UTF8, I need a conversion function to return proper UTF8
>> - if (origin_encoding != 8 && !ConvertFunction)
>> + if (origin_encoding != 8 && !options.GetConversionFunction())
>> + return false;
>> +
>> + if (!options.GetStream())
>> return false;
>>
>> + uint32_t sourceSize = options.GetSourceSize();
>> + bool needs_zero_terminator = options.GetNeedsZeroTermination();
>> +
>> if (!sourceSize)
>> + {
>> sourceSize =
>> process_sp->GetTarget().GetMaximumSizeOfStringSummary();
>> + needs_zero_terminator = true;
>> + }
>> else
>> sourceSize =
>> std::min(sourceSize,process_sp->GetTarget().GetMaximumSizeOfStringSummary(
>> ));
>>
>> @@ -287,16 +426,21 @@ ReadUTFBufferAndDumpToStream (Conversion
>> Error error;
>> char *buffer = reinterpret_cast<char *>(buffer_sp->GetBytes());
>>
>> - size_t data_read = process_sp->ReadStringFromMemory(location,
>> buffer, bufferSPSize, error, type_width);
>> + size_t data_read = 0;
>> + if (needs_zero_terminator)
>> + data_read =
>> process_sp->ReadStringFromMemory(options.GetLocation(), buffer,
>> bufferSPSize, error, type_width);
>> + else
>> + data_read =
>> process_sp->ReadMemoryFromInferior(options.GetLocation(),
>> (char*)buffer_sp->GetBytes(), bufferSPSize, error);
>> +
>> if (error.Fail() || data_read == 0)
>> {
>> - stream.Printf("unable to read data");
>> + options.GetStream()->Printf("unable to read data");
>> return true;
>> }
>>
>> DataExtractor data(buffer_sp, process_sp->GetByteOrder(),
>> process_sp->GetAddressByteSize());
>>
>> - return DumpUTFBufferToStream(ConvertFunction, data, stream,
>> prefix_token, quote, sourceSize);
>> + return DumpUTFBufferToStream(options.GetConversionFunction(), data,
>> *options.GetStream(), options.GetPrefixToken(), options.GetQuote(),
>> sourceSize);
>> }
>>
>> bool
>> @@ -311,10 +455,14 @@ lldb_private::formatters::Char16StringSu
>> if (!valobj_addr)
>> return false;
>>
>> - if
>> (!ReadUTFBufferAndDumpToStream<UTF16>(ConvertUTF16toUTF8,valobj_addr,
>> -
>> process_sp,
>> - stream,
>> - 'u'))
>> + ReadUTFBufferAndDumpToStreamOptions<UTF16> options;
>> + options.SetLocation(valobj_addr);
>> + options.SetConversionFunction(ConvertUTF16toUTF8);
>> + options.SetProcessSP(process_sp);
>> + options.SetStream(&stream);
>> + options.SetPrefixToken('u');
>> +
>> + if (!ReadUTFBufferAndDumpToStream(options))
>> {
>> stream.Printf("Summary Unavailable");
>> return true;
>> @@ -335,10 +483,14 @@ lldb_private::formatters::Char32StringSu
>> if (!valobj_addr)
>> return false;
>>
>> - if
>> (!ReadUTFBufferAndDumpToStream<UTF32>(ConvertUTF32toUTF8,valobj_addr,
>> -
>> process_sp,
>> - stream,
>> - 'U'))
>> + ReadUTFBufferAndDumpToStreamOptions<UTF32> options;
>> + options.SetLocation(valobj_addr);
>> + options.SetConversionFunction(ConvertUTF32toUTF8);
>> + options.SetProcessSP(process_sp);
>> + options.SetStream(&stream);
>> + options.SetPrefixToken('u');
>> +
>> + if (!ReadUTFBufferAndDumpToStream(options))
>> {
>> stream.Printf("Summary Unavailable");
>> return true;
>> @@ -374,23 +526,42 @@ lldb_private::formatters::WCharStringSum
>> switch (wchar_size)
>> {
>> case 8:
>> + {
>> // utf 8
>> - return ReadUTFBufferAndDumpToStream<UTF8>(nullptr, data_addr,
>> -
>> process_sp,
>> - stream,
>> - 'L');
>> +
>> + ReadUTFBufferAndDumpToStreamOptions<UTF8> options;
>> + options.SetLocation(data_addr);
>> + options.SetConversionFunction(nullptr);
>> + options.SetProcessSP(process_sp);
>> + options.SetStream(&stream);
>> + options.SetPrefixToken('L');
>> +
>> + return ReadUTFBufferAndDumpToStream(options);
>> + }
>> case 16:
>> + {
>> // utf 16
>> - return
>> ReadUTFBufferAndDumpToStream<UTF16>(ConvertUTF16toUTF8, data_addr,
>> -
>> process_sp,
>> -
>> stream,
>> -
>> 'L');
>> + ReadUTFBufferAndDumpToStreamOptions<UTF16> options;
>> + options.SetLocation(data_addr);
>> + options.SetConversionFunction(ConvertUTF16toUTF8);
>> + options.SetProcessSP(process_sp);
>> + options.SetStream(&stream);
>> + options.SetPrefixToken('L');
>> +
>> + return ReadUTFBufferAndDumpToStream(options);
>> + }
>> case 32:
>> + {
>> // utf 32
>> - return
>> ReadUTFBufferAndDumpToStream<UTF32>(ConvertUTF32toUTF8, data_addr,
>> -
>> process_sp,
>> -
>> stream,
>> -
>> 'L');
>> + ReadUTFBufferAndDumpToStreamOptions<UTF32> options;
>> + options.SetLocation(data_addr);
>> + options.SetConversionFunction(ConvertUTF32toUTF8);
>> + options.SetProcessSP(process_sp);
>> + options.SetStream(&stream);
>> + options.SetPrefixToken('L');
>> +
>> + return ReadUTFBufferAndDumpToStream(options);
>> + }
>> default:
>> stream.Printf("size for wchar_t is not valid");
>> return true;
>> @@ -825,7 +996,7 @@ lldb_private::formatters::NSStringSummar
>> #endif
>> void* contentsAllocator = %p
>> }
>> - } variants;
>> + } variants; ==> (M:%dI:%dL:%zuU:%dS:%dN:%d)
>> };\n)",
>> my_string_data._cfisa,
>>
>> my_string_data._cfinfo[0],my_string_data._cfinfo[1],my_string_data._cfinfo
>> [2],my_string_data._cfinfo[3],
>> @@ -845,7 +1016,13 @@ lldb_private::formatters::NSStringSummar
>> my_string_data.variants.notInlineMutable.capacityProvidedExternally,
>> my_string_data.variants.notInlineMutable.desiredCapacity,
>> my_string_data.variants.notInlineMutable.desiredCapacity,
>> - my_string_data.variants.notInlineMutable.contentsAllocator);
>> + my_string_data.variants.notInlineMutable.contentsAllocator,
>> + is_mutable,
>> + is_inline,
>> + explicit_length,
>> + is_unicode,
>> + is_special,
>> + has_null);
>> #endif
>>
>> if (strcmp(class_name,"NSString") &&
>> @@ -862,16 +1039,6 @@ lldb_private::formatters::NSStringSummar
>> return true;
>> }
>>
>> -#ifdef WANT_DEEP_PRNT
>> - stream.Printf("(M:%dI:%dL:%zuU:%dS:%dN:%d) ",
>> - is_mutable,
>> - is_inline,
>> - explicit_length,
>> - is_unicode,
>> - is_special,
>> - has_null);
>> -#endif
>> -
>> if (is_mutable)
>> {
>> uint64_t location = 2 * ptr_size + valobj_addr;
>> @@ -879,7 +1046,18 @@ lldb_private::formatters::NSStringSummar
>> if (error.Fail())
>> return false;
>> if (has_explicit_length and is_unicode)
>> - return ReadUTFBufferAndDumpToStream<UTF16>
>> (ConvertUTF16toUTF8,location, process_sp, stream, '@', '"',
>> explicit_length);
>> + {
>> + ReadUTFBufferAndDumpToStreamOptions<UTF16> options;
>> + options.SetConversionFunction(ConvertUTF16toUTF8);
>> + options.SetLocation(location);
>> + options.SetProcessSP(process_sp);
>> + options.SetStream(&stream);
>> + options.SetPrefixToken('@');
>> + options.SetQuote('"');
>> + options.SetSourceSize(explicit_length);
>> + options.SetNeedsZeroTermination(false);
>> + return ReadUTFBufferAndDumpToStream (options);
>> + }
>> else
>> return
>> ReadAsciiBufferAndDumpToStream(location+1,process_sp,stream,
>> explicit_length);
>> }
>> @@ -900,19 +1078,37 @@ lldb_private::formatters::NSStringSummar
>> }
>> else
>> location += ptr_size;
>> - }
>> + }
>> else
>> {
>> location = process_sp->ReadPointerFromMemory(location,
>> error);
>> if (error.Fail())
>> return false;
>> }
>> - return ReadUTFBufferAndDumpToStream<UTF16> (ConvertUTF16toUTF8,
>> location, process_sp, stream, '@', '"', explicit_length);
>> + ReadUTFBufferAndDumpToStreamOptions<UTF16> options;
>> + options.SetConversionFunction(ConvertUTF16toUTF8);
>> + options.SetLocation(location);
>> + options.SetProcessSP(process_sp);
>> + options.SetStream(&stream);
>> + options.SetPrefixToken('@');
>> + options.SetQuote('"');
>> + options.SetSourceSize(explicit_length);
>> + options.SetNeedsZeroTermination(has_explicit_length == false);
>> + return ReadUTFBufferAndDumpToStream (options);
>> }
>> else if (is_special)
>> {
>> uint64_t location = valobj_addr + (ptr_size == 8 ? 12 : 8);
>> - return ReadUTFBufferAndDumpToStream<UTF16> (ConvertUTF16toUTF8,
>> location, process_sp, stream, '@', '"', explicit_length);
>> + ReadUTFBufferAndDumpToStreamOptions<UTF16> options;
>> + options.SetConversionFunction(ConvertUTF16toUTF8);
>> + options.SetLocation(location);
>> + options.SetProcessSP(process_sp);
>> + options.SetStream(&stream);
>> + options.SetPrefixToken('@');
>> + options.SetQuote('"');
>> + options.SetSourceSize(explicit_length);
>> + options.SetNeedsZeroTermination(has_explicit_length == false);
>> + return ReadUTFBufferAndDumpToStream (options);
>> }
>> else if (is_inline)
>> {
>>
>> Modified: lldb/trunk/test/redo.py
>> URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/redo.py?rev=180141&r1=
>> 180140&r2=180141&view=diff
>> ==========================================================================
>> ====
>> --- lldb/trunk/test/redo.py (original)
>> +++ lldb/trunk/test/redo.py Tue Apr 23 15:05:05 2013
>> @@ -28,6 +28,8 @@ redo_specs = []
>> # will be considered for re-run. Examples: ['X86_64', 'clang'].
>> filename_components = []
>>
>> +do_delay = False
>> +
>> # There is a known bug with respect to comp_specs and arch_specs, in
>> that if we
>> # encountered "-C clang" and "-C gcc" when visiting the session files,
>> both
>> # compilers will end up in the invocation of the test driver when
>> rerunning.
>> @@ -40,12 +42,13 @@ arch_specs = set()
>>
>> def usage():
>> print"""\
>> -Usage: redo.py [-F filename_component] [-n] [session_dir]
>> +Usage: redo.py [-F filename_component] [-n] [session_dir] [-d]
>> where options:
>> -F : only consider the test for re-run if the session filename conatins
>> the filename component
>> for example: -F x86_64
>> -n : when running the tests, do not turn on trace mode, i.e, no '-t'
>> option
>> is passed to the test driver (this will run the tests faster)
>> +-d : pass -d down to the test driver (introduces a delay so you can
>> attach with a debugger)
>>
>> and session_dir specifies the session directory which contains previously
>> recorded session infos for all the test cases which either failed or
>> errored.
>> @@ -81,6 +84,7 @@ def redo(suffix, dir, names):
>> global comp_pattern
>> global arch_pattern
>> global filename_components
>> + global do_delay
>>
>> for name in names:
>> if name.endswith(suffix):
>> @@ -111,6 +115,7 @@ def main():
>> global no_trace
>> global redo_specs
>> global filename_components
>> + global do_delay
>>
>> test_dir = sys.path[0]
>> if not test_dir:
>> @@ -139,6 +144,8 @@ def main():
>> filename_components.append(sys.argv[index])
>> elif sys.argv[index] == '-n':
>> no_trace = True
>> + elif sys.argv[index] == '-d':
>> + do_delay = True
>>
>> index += 1
>>
>> @@ -176,7 +183,7 @@ def main():
>> for arch in arch_specs:
>> archs += "--arch %s " % (arch)
>>
>> - command = "./dotest.py %s %s -v %s -f " % (compilers, archs, "" if
>> no_trace else "-t")
>> + command = "./dotest.py %s %s -v %s %s -f " % (compilers, archs, ""
>> if no_trace else "-t", "-d" if do_delay else "")
>>
>>
>> print "Running %s" % (command + filters)
>>
>>
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130423/53108d47/attachment.html>
More information about the lldb-commits
mailing list