This is an archive of the discontinued LLVM Phabricator instance.

[lldb/test] Fix for flakiness in TestNSDictionarySynthetic
ClosedPublic

Authored by vsk on May 8 2020, 11:56 AM.

Details

Summary

TestNSDictionarySynthetic sets up an NSURL which does not initialize its
_baseURL member. When the test runs and we print out the NSURL, we print
out some garbage memory pointed-to by the _baseURL member, like:

_baseURL = 0x0800010020004029 @"d��qX"

and this can cause a python unicode decoding error like:

UnicodeDecodeError: 'utf8' codec can't decode byte 0xa0 in position
10309: invalid start byte

There's a discrepancy here because lldb's StringPrinter facility tries
to only print out "printable" sequences (see: isprint32()), whereas python
rejects the StringPrinter output as invalid utf8. For the specific error
seen above, lldb's isprint32(0xa0) = true, even though 0xa0 is not
really "printable" in the usual sense.

The problem is that lldb and python disagree on what exactly is
"printable". Both have dismayingly hand-rolled utf8 validation code
(c.f. _Py_DecodeUTF8Ex), and I can't really tell which one is more
correct.

I tried replacing lldb's isprint32() with a call to libc's iswprint():
this satisfied python, but broke emoji printing :|.

Now, I believe that lldb (and python too) ought to just call into some
battle-tested utf library, and that we shouldn't aim for compatibility
with python's strict unicode decoding mode until then.

FWIW I ran this test under an ASanified lldb hundreds of times but
didn't turn up any other issues.

rdar://62941711

Diff Detail

Event Timeline

vsk created this revision.May 8 2020, 11:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2020, 11:56 AM
JDevlieghere accepted this revision.May 8 2020, 3:17 PM

LGTM. We discussed adding this during the original review, but I left it out because we could always add it if we needed it. Turns out we do!

This revision is now accepted and ready to land.May 8 2020, 3:17 PM
shafik added inline comments.May 8 2020, 4:06 PM
lldb/test/API/lldbtest.py
122–129

Maybe add a comment as to why we are using 'replace' and LLDB and python implementation disagree and this works for now.

shafik accepted this revision.May 8 2020, 4:07 PM
vsk marked 2 inline comments as done.May 11 2020, 9:53 AM
vsk added inline comments.
lldb/test/API/lldbtest.py
122–129

Definitely, I'll add this before committing.

This revision was automatically updated to reflect the committed changes.
vsk marked an inline comment as done.