[Lldb-commits] [lldb] r180141 - The new ReadStringFromMemory() API does not work correctly with NSStrings that have an explicit length and no NULL terminator
Malea, Daniel
daniel.malea at intel.com
Tue Apr 23 13:37:05 PDT 2013
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
More information about the lldb-commits
mailing list