<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 12 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:"MS Mincho";
panose-1:2 2 6 9 4 2 5 8 3 4;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
@font-face
{font-family:"\@MS Mincho";
panose-1:2 2 6 9 4 2 5 8 3 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.MsoAcetate, li.MsoAcetate, div.MsoAcetate
{mso-style-priority:99;
mso-style-link:"Balloon Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:8.0pt;
font-family:"Tahoma","sans-serif";}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
span.apple-converted-space
{mso-style-name:apple-converted-space;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.BalloonTextChar
{mso-style-name:"Balloon Text Char";
mso-style-priority:99;
mso-style-link:"Balloon Text";
font-family:"Tahoma","sans-serif";}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
/* List Definitions */
@list l0
{mso-list-id:1896895407;
mso-list-type:hybrid;
mso-list-template-ids:1492918844 843747878 67698691 67698693 67698689 67698691 67698693 67698689 67698691 67698693;}
@list l0:level1
{mso-level-start-at:0;
mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Wingdings;
mso-fareast-font-family:Calibri;
mso-bidi-font-family:"Times New Roman";}
@list l1
{mso-list-id:1927230359;
mso-list-type:hybrid;
mso-list-template-ids:978740506 -826737890 67698691 67698693 67698689 67698691 67698693 67698689 67698691 67698693;}
@list l1:level1
{mso-level-start-at:0;
mso-level-number-format:bullet;
mso-level-text:-;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Calibri","sans-serif";
mso-fareast-font-family:Calibri;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoListParagraph" style="text-indent:-.25in;mso-list:l0 level1 lfo1"><![if !supportLists]><span style="font-family:Wingdings"><span style="mso-list:Ignore">Ø<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]>Feel free to change your patch in this way and check that in. We will then take it from there.<o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Now that the Linux buildbots provide some test coverage, I committed ReadStringFromMemory in r179857 and included a comment for the deprecated sibling. I hooked
up the new API to ReadUTFBufferAndDumpToStream, which fixes wchar failures on Linux on the buildbots. Once stable, I assume we want a carbon copy in Target. Cheers,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoListParagraph" style="text-indent:-.25in;mso-list:l1 level1 lfo2"><![if !supportLists]><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><span style="mso-list:Ignore">-<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Ashok<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Enrico Granata [mailto:egranata@apple.com]
<br>
<b>Sent:</b> Thursday, April 11, 2013 5:06 PM<br>
<b>To:</b> Thirumurthi, Ashok<br>
<b>Cc:</b> lldb-dev@cs.uiuc.edu<br>
<b>Subject:</b> Re: [lldb-dev] PATCH for REVIEW - Fix [Bug 15038] LLDB does not support printing wide-character variables on Linux<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Hey there,<o:p></o:p></p>
<div>
<p class="MsoNormal">sorry for the delay. I have been busy over the last couple of days with some high priority work.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">I have discussed this idea with a coworker. Our idea is that your “extended reader” is a good idea.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">What we would prefer to see is committing it as a new function (e.g. Process::ReadNullTerminatedStringFromMemory) - that would allow an experimentation phase where we can use the old and new function to make sure things work correctly with
it. Eventually, once we are all fully satisfied with the new API, the existing CString reader would simply become a wrapper for the NullTerminated reader passing a size of 1.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Feel free to change your patch in this way and check that in.We will then take it from there.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
</div>
<div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:7.0pt;color:black">Enrico Granata<br>
</span><span style="font-size:7.0pt;font-family:"MS Mincho";color:black">✉</span><span style="font-size:7.0pt;color:black">
<a href="mailto:egranata@.com">egranata@.com</a><br>
</span><span style="font-size:7.0pt;font-family:"MS Mincho";color:black">✆</span><span style="font-size:7.0pt;color:black"> 27683</span><span style="font-size:13.5pt;color:black"><o:p></o:p></span></p>
</div>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Apr 11, 2013, at 1:09 PM, "Thirumurthi, Ashok" <<a href="mailto:ashok.thirumurthi@intel.com">ashok.thirumurthi@intel.com</a>> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Ping!</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<div>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span class="apple-converted-space"><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> </span></span><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""><a href="mailto:lldb-dev-bounces@cs.uiuc.edu">lldb-dev-bounces@cs.uiuc.edu</a>
[<a href="mailto:lldb-dev-bounces@cs.uiuc.edu">mailto:lldb-dev-bounces@cs.uiuc.edu</a>]<span class="apple-converted-space"> </span><b>On Behalf Of<span class="apple-converted-space"> </span></b>Thirumurthi, Ashok<br>
<b>Sent:</b><span class="apple-converted-space"> </span>Tuesday, April 02, 2013 10:20 PM<br>
<b>To:</b><span class="apple-converted-space"> </span>Enrico Granata<br>
<b>Cc:</b><span class="apple-converted-space"> </span><a href="mailto:lldb-dev@cs.uiuc.edu">lldb-dev@cs.uiuc.edu</a><br>
<b>Subject:</b><span class="apple-converted-space"> </span>Re: [lldb-dev] PATCH for REVIEW - Fix [Bug 15038] LLDB does not support printing wide-character variables on Linux</span><o:p></o:p></p>
</div>
</div>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Hi Enrico,</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Your suggestions got me thinking that the version of ReadCStringFromMemory that takes a char* can be extended to support null terminators of any width. The
attached patch is therefore a variation on what you suggested that consolidates the read and validation into one method, and handles strings of any type width. In addition, this implementation provides a null terminator when a partial read occurs.</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Incidentally, in the old implementation, strlen would always succeed on the first call, because curr_dst is set to dst, which is memset to 0. The current implementation
will scan for a null terminator within bytes_read and stop reading when one is found.</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">If this is the right approach, let me know if I should modify SBProcess to match (i.e to add the new parameter type_width). Also, I noticed that this code
is largely duplicated in methods of the same name in class Target. Can I assume that a complete patch would include changes to all of those methods? Thanks in advance,</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div style="margin-left:.5in">
<p class="MsoNormal" style="text-indent:-.25in"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">-</span><span style="font-size:7.0pt;color:#1F497D"> <span class="apple-converted-space"> </span></span><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Ashok</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<div>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span class="apple-converted-space"><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> </span></span><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">Enrico
Granata [<a href="mailto:egranata@apple.com"><span style="color:purple">mailto:egranata@apple.com</span></a>]<span class="apple-converted-space"> </span><br>
<b>Sent:</b><span class="apple-converted-space"> </span>Monday, April 01, 2013 6:49 PM<br>
<b>To:</b><span class="apple-converted-space"> </span>Thirumurthi, Ashok<br>
<b>Cc:</b><span class="apple-converted-space"> </span><a href="mailto:lldb-dev@cs.uiuc.edu"><span style="color:purple">lldb-dev@cs.uiuc.edu</span></a><br>
<b>Subject:</b><span class="apple-converted-space"> </span>Re: [lldb-dev] PATCH for REVIEW - Fix [Bug 15038] LLDB does not support printing wide-character variables on Linux</span><o:p></o:p></p>
</div>
</div>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal">On Apr 1, 2013, at 2:50 PM, "Thirumurthi, Ashok" <<a href="mailto:ashok.thirumurthi@intel.com"><span style="color:purple">ashok.thirumurthi@intel.com</span></a>> wrote:<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt"> <o:p></o:p></p>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">Hi Enrico,</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div style="margin-left:.5in">
<div>
<p class="MsoNormal" style="text-indent:-.25in"><span style="font-size:14.0pt;font-family:Wingdings">Ø</span><span class="apple-converted-space"><span style="font-size:7.0pt"> </span></span><span style="font-size:14.0pt">If we go this route, the same thing
might have to be done for ValueObject::ReadPointedString() which does the same kind of “unbounded read”</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D">I see. Looking through the code base for methods that call GetMaximumSizeOfStringSummary, I see that there are a number of code paths to handle. The attached patch lowers
the fix so that this functionality can be used as needed via a helper method in Process to validate an unbounded read.</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">We already have a Process::ReadCStringFromMemory - that does read&validation<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">You might instead want to consider making a Process::ReadWStringFromMemory<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">The mere act of validating that a buffer is 0-terminated is not a Process specific operation at all<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div style="margin-left:.5in">
<div>
<p class="MsoNormal" style="text-indent:-.25in"><span style="font-family:Wingdings;color:#1F497D">Ø</span><span style="font-size:7.0pt;color:#1F497D"> <span class="apple-converted-space"> </span></span><span style="font-size:14.0pt">But I am inclined to believe
the best option would be to adopt your ProcessMonitor fix that reads smaller and smaller chunks and then returns the number of bytes read and no error</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D">Well, the unbounded read is a characteristic of the use case (i.e. string reads), not the implementation. The behavior of the plug-in under this type of request is implementation
specific, and an error is always reasonable since there was a failure to read the requested size in bytes. My point is that the corner case of null terminator in the last 8-byte read needs to be tested and fixed.</span><o:p></o:p></p>
</div>
</div>
</blockquote>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
<div style="margin-left:.5in">
<div>
<p class="MsoNormal" style="text-indent:-.25in"><span style="font-size:14.0pt;font-family:Wingdings">Ø</span><span class="apple-converted-space"><span style="font-size:7.0pt"> </span></span><span style="font-size:14.0pt">What troubles me a little with the overall
idea of this patch is that we are putting a workaround for a lower-level issue in higher-level code.</span><o:p></o:p></p>
</div>
</div>
<div style="margin-left:.5in">
<div>
<p class="MsoNormal" style="text-indent:-.25in"><span style="font-size:14.0pt;font-family:Wingdings">Ø</span><span class="apple-converted-space"><span style="font-size:7.0pt"> </span></span><span style="font-size:14.0pt">I would prefer, if we do this, that
at least we put the code around #if defined (__linux__), as in:</span><o:p></o:p></p>
</div>
</div>
<div style="margin-left:.5in">
<div>
<p class="MsoNormal" style="text-indent:-.25in"><span style="font-size:14.0pt;font-family:Wingdings">Ø</span><span class="apple-converted-space"><span style="font-size:7.0pt"> </span></span><span style="font-size:14.0pt">What we have here is a Linux-specific
implementation issue</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D">Note that the issue can appear with any ptrace implementation. So, I lowered the fix to the ProcessPOSIX plugin, which is consistent with the case of eErrorTypePOSIX.</span><o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">I don’t think that is a Process responsibility to validate a buffer<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">If you want to make it a new type of request, ReadWString, and pass it a size so it knows how many consecutive bytes must be 0 to mean EndOfData, that would be fine.<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">It would look like<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">size_t Process::ReadWStringFromInferior(const void* buffer, size_t max_size, Error& error, size_t sizeofitem);<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">and there you could check the end-of-string<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">Then the WString data formatter can be moved to use this new read request, but you will have to ensure that partial strings are accepted :-)<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:7.0pt">Enrico Granata<br>
</span><span style="font-size:7.0pt;font-family:"MS Mincho"">✉</span><span class="apple-converted-space"><span style="font-size:7.0pt"> </span></span><span style="font-size:7.0pt"><a href="mailto:egranata@"><span style="color:purple">egranata@.com</span></a><br>
</span><span style="font-size:7.0pt;font-family:"MS Mincho"">✆</span><span class="apple-converted-space"><span style="font-size:7.0pt"> </span></span><span style="font-size:7.0pt">27683</span><o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"> <o:p></o:p></p>
</div>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D">Thanks again for the feedback,</span><o:p></o:p></p>
</div>
</div>
<div style="margin-left:.5in">
<div>
<p class="MsoNormal" style="text-indent:-.25in"><span style="font-family:"Calibri","sans-serif";color:#1F497D">-</span><span style="font-size:7.0pt;color:#1F497D"> <span class="apple-converted-space"> </span></span><span style="font-family:"Calibri","sans-serif";color:#1F497D">Ashok</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in;z-index:auto">
<div>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span class="apple-converted-space"><span style="font-size:11.0pt;font-family:"Tahoma","sans-serif""> </span></span><span style="font-size:11.0pt;font-family:"Tahoma","sans-serif"">Enrico
Granata [<a href="mailto:egranata@apple.com"><span style="color:purple">mailto:egranata@apple.com</span></a>]<span class="apple-converted-space"> </span><br>
<b>Sent:</b><span class="apple-converted-space"> </span>Thursday, March 28, 2013 2:39 PM<br>
<b>To:</b><span class="apple-converted-space"> </span>Thirumurthi, Ashok<br>
<b>Cc:</b><span class="apple-converted-space"> </span><a href="mailto:lldb-dev@cs.uiuc.edu"><span style="color:purple">lldb-dev@cs.uiuc.edu</span></a><br>
<b>Subject:</b><span class="apple-converted-space"> </span>Re: [lldb-dev] PATCH for REVIEW - Fix [Bug 15038] LLDB does not support printing wide-character variables on Linux</span><o:p></o:p></p>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">Hi,</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">now I pretty much see where you are coming from.</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">What we have here is a Linux-specific implementation issue (which of course is not going to get fixed anytime soon :-)</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">What troubles me a little with the overall idea of this patch is that we are putting a workaround for a lower-level issue in higher-level code.</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">If we go this route, the same thing might have to be done for ValueObject::ReadPointedString() which does the same kind of “unbounded read”</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">I would prefer, if we do this, that at least we put the code around #if defined (__linux__), as in:</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> if (error.Fail() || data_read == 0)</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> {</span><o:p></o:p></p>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ bool found = false;</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">#if defined (__linux__)</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ if (data_read && error.GetType() == eErrorTypePOSIX) {</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ // Determine if a null terminator was found in spite of an out-of-bounds read</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ if (error.GetError() == EIO || error.GetError() == EFAULT) {</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ char* buffer = reinterpret_cast<char *>(buffer_sp->GetBytes());</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ char terminator[4] = {'\0', '\0', '\0', '\0'};</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ assert(sizeof(terminator) >= type_size && "Atempting to read a wchar with more than 4 bytes per character!");</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ for (size_t i = 0; !found && (i + type_size <= data_read); i += type_size)</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ if (::strncmp(&buffer[i], terminator, type_size) == 0)</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ found = true; // null terminator</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ }</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ }</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">#endif</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ if (!found) {</span><o:p></o:p></p>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ stream.Printf("unable to read data");</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">+ return true;</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> }</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">}</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">This will maintain a clean behavior for non-Linux systems (and other systems with a similarly broken API can opt-in this behavior by adding themselves to the ifdef)</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">But I am inclined to believe the best option would be to adopt your ProcessMonitor fix that reads smaller and smaller chunks and then returns the number of bytes read and no error - that would make all unbounded-read-based
calls just work without having to add Linux-specific patches around (main issue being that now we all have to remember that anything that reads a string-like thing has to add a similar workaround or cause Linux unhappiness)</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">If that is not going to be viable, or will take some time, I don’t want to keep you blocked, so I would suggest:</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">- commit this patch with #ifdef markers to keep it Linux-only</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">- file a bugzilla issue to make process reads as functional as possible on Linux</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">Thoughts?</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:8.0pt">Enrico Granata<br>
</span><span style="font-size:8.0pt;font-family:"MS Mincho"">✉</span><span class="apple-converted-space"><span style="font-size:8.0pt"> </span></span><span style="font-size:8.0pt"><a href="mailto:egranata@"><span style="color:purple">egranata@.com</span></a><br>
</span><span style="font-size:8.0pt;font-family:"MS Mincho"">✆</span><span class="apple-converted-space"><span style="font-size:8.0pt"> </span></span><span style="font-size:8.0pt">27683</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">On Mar 28, 2013, at 8:39 AM, "Thirumurthi, Ashok" <<a href="mailto:ashok.thirumurthi@intel.com"><span style="color:purple">ashok.thirumurthi@intel.com</span></a>> wrote:</span><o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:14.0pt"><br>
<br>
<br>
</span><o:p></o:p></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D">Thanks for the feedback, Enrico,</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D">I’m relying on the fact that when the wstring is larger than the default size (1GB in your example), we won’t get a ptrace error like EIO or EFAULT. So, there will be no search
for a null terminator.</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D">If we can’t rely on the null terminator, it’s hard to know if a different error occurred. The trouble is that there is inconsistency in the PTRACE_PEEK implementation on Linux
that is more or less arbitrary (sigh), so the out-of-bounds read can generate EIO or EFAULT. In particular, EIO can mean a number of things:</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D"> “</span><i><span style="font-size:9.0pt;font-family:"Arial","sans-serif";background:white">request</span></i><span class="apple-converted-space"><span style="font-size:9.0pt;font-family:"Arial","sans-serif";background:white"> </span></span><span style="font-size:9.0pt;font-family:"Arial","sans-serif";background:white">is
invalid, or an attempt was made to read from or write to an invalid area in the parent's or child's memory, or there was a word-alignment violation, or an invalid signal was specified during a restart request.</span><span style="font-family:"Calibri","sans-serif";color:#1F497D">”
–<a href="http://linux.die.net/man/2/ptrace"><span style="color:purple">http://linux.die.net/man/2/ptrace</span></a> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D">In addition, the Linux ProcessMonitor reads in 8-byte chunks, so ReadUTFBufferAndDumpToStream will probably fail (for the right reasons) in the case where at least part of the
null terminator was in that block. I see that as an implementation detail that can be resolved in ProcessMonitor by reading smaller chunks until PTRACE_PEEK succeeds (i.e. a good follow-up patch).</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D">Do let me know if you feel this is okay to commit as is. Cheers,</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
<div style="margin-left:.5in">
<div>
<p class="MsoNormal" style="text-indent:-.25in"><span style="font-family:"Calibri","sans-serif";color:#1F497D">-</span><span style="font-size:8.0pt;color:#1F497D"> <span class="apple-converted-space"> </span></span><span style="font-family:"Calibri","sans-serif";color:#1F497D">Ashok</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<div>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span class="apple-converted-space"><span style="font-size:11.0pt;font-family:"Tahoma","sans-serif""> </span></span><span style="font-size:11.0pt;font-family:"Tahoma","sans-serif"">Enrico
Granata [<a href="mailto:egranata@apple.com"><span style="color:purple">mailto:egranata@apple.com</span></a>]<span class="apple-converted-space"> </span><br>
<b>Sent:</b><span class="apple-converted-space"> </span>Wednesday, March 27, 2013 6:18 PM<br>
<b>To:</b><span class="apple-converted-space"> </span>Thirumurthi, Ashok<br>
<b>Cc:</b><span class="apple-converted-space"> </span><a href="mailto:lldb-dev@cs.uiuc.edu"><span style="color:purple">lldb-dev@cs.uiuc.edu</span></a><br>
<b>Subject:</b><span class="apple-converted-space"> </span>Re: [lldb-dev] PATCH for REVIEW - Fix [Bug 15038] LLDB does not support printing wide-character variables on Linux</span><o:p></o:p></p>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">Hi Ashok,</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">thanks for submitting an LLDB patch :)</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">I have looked at the code and while I see where the patch is coming from, I see a potential issue with it.</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">The code in ReadUTFBufferAndDumpToStream, which is what you are editing, is meant to work on a partial string (i.e. one that does not have a NULL terminator at the end). The reason for that is that you might
have a wstring that is 1GB long and we would not want to try and read all of it and then display it. What we do is pick a size and only extract that much data. For obvious reasons, your string might be longer than our upper boundary, so you would get a chunk
of valid bytes and then no end-of-buffer marker.</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">It looks like your code would fail in that case and produce no summary for a string if an EIO was received before \0.</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">Is there any reason why you can’t just check if (error == EIO and data_read > 0) and if so treat this as a “partial string” condition and keep going?</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">Would that break/crash anything?</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">Best,</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:8.0pt">Enrico Granata<br>
</span><span style="font-size:8.0pt;font-family:"MS Mincho"">✉</span><span class="apple-converted-space"><span style="font-size:8.0pt"> </span></span><span style="font-size:8.0pt"><a href="mailto:egranata@"><span style="color:purple">egranata@.com</span></a><br>
</span><span style="font-size:8.0pt;font-family:"MS Mincho"">✆</span><span class="apple-converted-space"><span style="font-size:8.0pt"> </span></span><span style="font-size:8.0pt">27683</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">On Mar 27, 2013, at 2:52 PM, "Thirumurthi, Ashok" <<a href="mailto:ashok.thirumurthi@intel.com"><span style="color:purple">ashok.thirumurthi@intel.com</span></a>> wrote:</span><o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:14.0pt"><br>
<br>
<br>
<br>
</span><o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt">The root cause for Bug 15038 is a ptrace EIO that can occur because we don't know the size of a (UTF) string and so read a fixed number of characters for strings. The attached fix accepts EIO in the case
where data has been read and a null terminator of the correct size and alignment was found.<br>
<br>
- Ashok<br>
<br>
-----Original Message-----<br>
From:<span class="apple-converted-space"> </span><a href="mailto:lldb-dev-bounces@cs.uiuc.edu"><span style="color:purple">lldb-dev-bounces@cs.uiuc.edu</span></a><span class="apple-converted-space"> </span>[<a href="mailto:lldb-dev-bounces@cs.uiuc.edu"><span style="color:purple">mailto:lldb-dev-bounces@cs.uiuc.edu</span></a>]
On Behalf Of<a href="mailto:bugzilla-daemon@llvm.org"><span style="color:purple">bugzilla-daemon@llvm.org</span></a><br>
Sent: Tuesday, January 22, 2013 10:13 AM<br>
To:<span class="apple-converted-space"> </span><a href="mailto:lldb-dev@cs.uiuc.edu"><span style="color:purple">lldb-dev@cs.uiuc.edu</span></a><br>
Subject: [lldb-dev] [Bug 15038] New: LLDB does not support printing wide-character variables on Linux<br>
<br>
<a href="http://llvm.org/bugs/show_bug.cgi?id=15038"><span style="color:purple">http://llvm.org/bugs/show_bug.cgi?id=15038</span></a><br>
<br>
Bug #: 15038<br>
Summary: LLDB does not support printing wide-character<br>
variables on Linux<br>
Product: lldb<br>
Version: unspecified<br>
Platform: PC<br>
OS/Version: Linux<br>
Status: NEW<br>
Severity: enhancement<br>
Priority: P<br>
Component: All Bugs<br>
AssignedTo:<span class="apple-converted-space"> </span><a href="mailto:lldb-dev@cs.uiuc.edu"><span style="color:purple">lldb-dev@cs.uiuc.edu</span></a><br>
ReportedBy:<span class="apple-converted-space"> </span><a href="mailto:daniel.malea@intel.com"><span style="color:purple">daniel.malea@intel.com</span></a><br>
Classification: Unclassified<br>
<br>
<br>
Printing a variable of wchar_t type does not behave as expected and results in garbage being printed on the screen.<br>
<br>
To reproduce, remove the @expectedFailureLinux decorator from TestChar1632T.py and TestCxxWCharT.py and run:<br>
<br>
python dotest.py --executable <path-to-lldb> lang/cpp/char1632_t lang/cpp/wchar_t<br>
<br>
--<br>
Configure bugmail:<span class="apple-converted-space"> </span><a href="http://llvm.org/bugs/userprefs.cgi?tab=email"><span style="color:purple">http://llvm.org/bugs/userprefs.cgi?tab=email</span></a><br>
------- You are receiving this mail because: ------- You are the assignee for the bug.<br>
_______________________________________________<br>
lldb-dev mailing list<br>
<a href="mailto:lldb-dev@cs.uiuc.edu"><span style="color:purple">lldb-dev@cs.uiuc.edu</span></a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev"><span style="color:purple">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev</span></a><br>
<read-strings.diff>_______________________________________________<br>
lldb-dev mailing list<br>
<a href="mailto:lldb-dev@cs.uiuc.edu"><span style="color:purple">lldb-dev@cs.uiuc.edu</span></a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev"><span style="color:purple">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev</span></a></span><o:p></o:p></p>
</div>
</div>
</div>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:14.0pt"> </span><o:p></o:p></p>
</div>
</div>
</div>
<div>
<p class="MsoNormal"><read-strings-2.diff><o:p></o:p></p>
</div>
</blockquote>
</div>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</body>
</html>