<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hey there,<div>sorry for the delay. I have been busy over the last couple of days with some high priority work.</div><div>I have discussed this idea with a coworker. Our idea is that your “extended reader” is a good idea.</div><div>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.</div><div>Feel free to change your patch in this way and check that in.We will then take it from there.</div><div><br></div><div>Thanks,</div><div><div>
<div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div style="font-size: medium; orphans: 2; widows: 2; border-collapse: separate; border-spacing: 0px;"><span style="font-size: 12px; orphans: auto; widows: auto;">Enrico Granata</span><br style="font-size: 12px; orphans: auto; widows: auto;"><span style="font-size: 12px; orphans: auto; widows: auto;">✉ egranata@</span><font color="#ff2600" style="font-size: 12px; orphans: auto; widows: auto;"></font><span style="font-size: 12px; orphans: auto; widows: auto;">.com</span><br style="font-size: 12px; orphans: auto; widows: auto;"><span style="font-size: 12px; orphans: auto; widows: auto;">✆ 27683</span></div></div>
</div>
<br><div><div>On Apr 11, 2013, at 1:09 PM, "Thirumurthi, Ashok" <<a href="mailto:ashok.thirumurthi@intel.com">ashok.thirumurthi@intel.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div lang="EN-US" link="blue" vlink="purple" style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="WordSection1" style="page: WordSection1;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Ping!<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div><div style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0in 0in;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;"><span class="Apple-converted-space"> </span><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<o:p></o:p></span></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Hi Enrico,<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">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.<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">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.<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">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,<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"><span>-<span style="font-style: normal; font-variant: normal; font-weight: normal; font-size: 7pt; line-height: normal; font-family: 'Times New Roman';"> <span class="Apple-converted-space"> </span></span></span></span><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Ashok<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span></div><div><div style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0in 0in;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;"><span class="Apple-converted-space"> </span>Enrico Granata [<a href="mailto:egranata@apple.com" style="color: purple; text-decoration: underline;">mailto:egranata@apple.com</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" style="color: purple; text-decoration: underline;">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<o:p></o:p></span></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p> </o:p></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">On Apr 1, 2013, at 2:50 PM, "Thirumurthi, Ashok" <<a href="mailto:ashok.thirumurthi@intel.com" style="color: purple; text-decoration: underline;">ashok.thirumurthi@intel.com</a>> wrote:<o:p></o:p></div></div><p class="MsoNormal" style="margin: 0in 0in 12pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p> </o:p></p><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">Hi Enrico,</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div style="margin-left: 0.5in;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span style="font-size: 14pt; font-family: Wingdings;">Ø</span><span class="apple-converted-space"><span style="font-size: 7pt;"> </span></span><span style="font-size: 14pt;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p> </o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">We already have a Process::ReadCStringFromMemory - that does read&validation<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">You might instead want to consider making a Process::ReadWStringFromMemory<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">The mere act of validating that a buffer is 0-terminated is not a Process specific operation at all<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p> </o:p></div></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;"><div style="margin-left: 0.5in;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span style="font-family: Wingdings; color: rgb(31, 73, 125);">Ø</span><span style="font-size: 7pt; color: rgb(31, 73, 125);"> <span class="apple-converted-space"> </span></span><span style="font-size: 14pt;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">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></div></div></blockquote><blockquote style="margin-top: 5pt; margin-bottom: 5pt;"><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div></div><div style="margin-left: 0.5in;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span style="font-size: 14pt; font-family: Wingdings;">Ø</span><span class="apple-converted-space"><span style="font-size: 7pt;"> </span></span><span style="font-size: 14pt;">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></div></div><div style="margin-left: 0.5in;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span style="font-size: 14pt; font-family: Wingdings;">Ø</span><span class="apple-converted-space"><span style="font-size: 7pt;"> </span></span><span style="font-size: 14pt;">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></div></div><div style="margin-left: 0.5in;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span style="font-size: 14pt; font-family: Wingdings;">Ø</span><span class="apple-converted-space"><span style="font-size: 7pt;"> </span></span><span style="font-size: 14pt;">What we have here is a Linux-specific implementation issue</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">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></div></div></blockquote><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p> </o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">I don’t think that is a Process responsibility to validate a buffer<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">It would look like<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">size_t Process::ReadWStringFromInferior(const void* buffer, size_t max_size, Error& error, size_t sizeofitem);<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">and there you could check the end-of-string<o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p> </o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">Thanks,<o:p></o:p></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 7pt;">Enrico Granata<br></span><span style="font-size: 7pt; font-family: 'MS Mincho';">✉</span><span style="font-size: 7pt;"><span class="Apple-converted-space"> </span><a href="mailto:egranata@" style="color: purple; text-decoration: underline;">egranata@.com</a><br></span><span style="font-size: 7pt; font-family: 'MS Mincho';">✆</span><span style="font-size: 7pt;"><span class="Apple-converted-space"> </span>27683</span><span style="font-size: 13.5pt;"><o:p></o:p></span></div></div><div><p class="MsoNormal" style="margin: 0in 0in 12pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p> </o:p></p></div></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;"><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Thanks again for the feedback,</span><o:p></o:p></div></div><div style="margin-left: 0.5in;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">-</span><span style="font-size: 7pt; color: rgb(31, 73, 125);"> <span class="apple-converted-space"> </span></span><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Ashok</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div></div><div><div style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0in 0in; z-index: auto;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><b><span style="font-size: 11pt; font-family: Tahoma, sans-serif;">From:</span></b><span class="apple-converted-space"><span style="font-size: 11pt; font-family: Tahoma, sans-serif;"> </span></span><span style="font-size: 11pt; font-family: Tahoma, sans-serif;">Enrico Granata [<a href="mailto:egranata@apple.com" style="color: purple; text-decoration: underline;">mailto:egranata@apple.com</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" style="color: purple; text-decoration: underline;">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></div></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">Hi,</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">now I pretty much see where you are coming from.</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">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></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> if (error.Fail() || data_read == 0)</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> {</span><o:p></o:p></div></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ bool found = false;</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">#if defined (__linux__)</span><o:p></o:p></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ if (data_read && error.GetType() == eErrorTypePOSIX) {</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ // Determine if a null terminator was found in spite of an out-of-bounds read</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ if (error.GetError() == EIO || error.GetError() == EFAULT) {</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ char* buffer = reinterpret_cast<char *>(buffer_sp->GetBytes());</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ char terminator[4] = {'\0', '\0', '\0', '\0'};</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ assert(sizeof(terminator) >= type_size && "Atempting to read a wchar with more than 4 bytes per character!");</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ for (size_t i = 0; !found && (i + type_size <= data_read); i += type_size)</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ if (::strncmp(&buffer[i], terminator, type_size) == 0)</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ found = true; // null terminator</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ }</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ }</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">#endif</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ if (!found) {</span><o:p></o:p></div></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ stream.Printf("unable to read data");</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">+ return true;</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> }</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">}</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">- commit this patch with #ifdef markers to keep it Linux-only</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">- file a bugzilla issue to make process reads as functional as possible on Linux</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">Thoughts?</span><o:p></o:p></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 8pt;">Enrico Granata<br></span><span style="font-size: 8pt; font-family: 'MS Mincho';">✉</span><span class="apple-converted-space"><span style="font-size: 8pt;"> </span></span><span style="font-size: 8pt;"><a href="mailto:egranata@" style="color: purple; text-decoration: underline;"><span style="color: purple;">egranata@.com</span></a><br></span><span style="font-size: 8pt; font-family: 'MS Mincho';">✆</span><span class="apple-converted-space"><span style="font-size: 8pt;"> </span></span><span style="font-size: 8pt;">27683</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">On Mar 28, 2013, at 8:39 AM, "Thirumurthi, Ashok" <<a href="mailto:ashok.thirumurthi@intel.com" style="color: purple; text-decoration: underline;"><span style="color: purple;">ashok.thirumurthi@intel.com</span></a>> wrote:</span><o:p></o:p></div></div><div><p class="MsoNormal" style="margin: 0in 0in 12pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"><br><br></span><o:p></o:p></p></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Thanks for the feedback, Enrico,</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> “</span><i><span style="font-size: 9pt; font-family: Arial, sans-serif; background-color: white; background-position: initial initial; background-repeat: initial initial;">request</span></i><span class="apple-converted-space"><span style="font-size: 9pt; font-family: Arial, sans-serif; background-color: white; background-position: initial initial; background-repeat: initial initial;"> </span></span><span style="font-size: 9pt; font-family: Arial, sans-serif; background-color: white; background-position: initial initial; background-repeat: initial initial;">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: rgb(31, 73, 125);">” –<a href="http://linux.die.net/man/2/ptrace" style="color: purple; text-decoration: underline;"><span style="color: purple;">http://linux.die.net/man/2/ptrace</span></a> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Do let me know if you feel this is okay to commit as is. Cheers,</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div></div><div style="margin-left: 0.5in;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif; text-indent: -0.25in;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">-</span><span style="font-size: 8pt; color: rgb(31, 73, 125);"> <span class="apple-converted-space"> </span></span><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);">Ashok</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-family: Calibri, sans-serif; color: rgb(31, 73, 125);"> </span><o:p></o:p></div></div><div><div style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0in 0in;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><b><span style="font-size: 11pt; font-family: Tahoma, sans-serif;">From:</span></b><span class="apple-converted-space"><span style="font-size: 11pt; font-family: Tahoma, sans-serif;"> </span></span><span style="font-size: 11pt; font-family: Tahoma, sans-serif;">Enrico Granata [<a href="mailto:egranata@apple.com" style="color: purple; text-decoration: underline;"><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" style="color: purple; text-decoration: underline;"><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></div></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">Hi Ashok,</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">thanks for submitting an LLDB patch :)</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">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></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">Would that break/crash anything?</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">Best,</span><o:p></o:p></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 8pt;">Enrico Granata<br></span><span style="font-size: 8pt; font-family: 'MS Mincho';">✉</span><span class="apple-converted-space"><span style="font-size: 8pt;"> </span></span><span style="font-size: 8pt;"><a href="mailto:egranata@" style="color: purple; text-decoration: underline;"><span style="color: purple;">egranata@.com</span></a><br></span><span style="font-size: 8pt; font-family: 'MS Mincho';">✆</span><span class="apple-converted-space"><span style="font-size: 8pt;"> </span></span><span style="font-size: 8pt;">27683</span><o:p></o:p></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div><div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">On Mar 27, 2013, at 2:52 PM, "Thirumurthi, Ashok" <<a href="mailto:ashok.thirumurthi@intel.com" style="color: purple; text-decoration: underline;"><span style="color: purple;">ashok.thirumurthi@intel.com</span></a>> wrote:</span><o:p></o:p></div></div><div><p class="MsoNormal" style="margin: 0in 0in 12pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"><br><br><br></span><o:p></o:p></p></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;">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" style="color: purple; text-decoration: underline;"><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" style="color: purple; text-decoration: underline;"><span style="color: purple;">mailto:lldb-dev-bounces@cs.uiuc.edu</span></a>] On Behalf Of<a href="mailto:bugzilla-daemon@llvm.org" style="color: purple; text-decoration: underline;"><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" style="color: purple; text-decoration: underline;"><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" style="color: purple; text-decoration: underline;"><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" style="color: purple; text-decoration: underline;"><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" style="color: purple; text-decoration: underline;"><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" style="color: purple; text-decoration: underline;"><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" style="color: purple; text-decoration: underline;"><span style="color: purple;">lldb-dev@cs.uiuc.edu</span></a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev" style="color: purple; text-decoration: underline;"><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" style="color: purple; text-decoration: underline;"><span style="color: purple;">lldb-dev@cs.uiuc.edu</span></a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev" style="color: purple; text-decoration: underline;"><span style="color: purple;">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev</span></a></span><o:p></o:p></div></div></div></div></div></div><div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 14pt;"> </span><o:p></o:p></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><read-strings-2.diff></div></blockquote></div></div></div></blockquote></div><br></div></body></html>