<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Dec 23, 2008, at 10:41 PM, Zhongxing Xu wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><br><br><div class="gmail_quote">On Wed, Dec 24, 2008 at 2:16 PM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> <div style=""><div><div></div><div class="Wj3C7c"><br><div><div>On Dec 23, 2008, at 5:01 PM, Zhongxing Xu wrote:</div><br><blockquote type="cite"><br><br><div class="gmail_quote">On Wed, Dec 24, 2008 at 7:29 AM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com" target="_blank">kremenek@apple.com</a>></span> wrote:<br> <blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> Part of the problem is that "Deserialize.cpp" (in llvm) uses GetCurrentBitNo() in a whole bunch of places. AST serialization/deserialization is the only client of the serialization library, and there aren't that many test cases in the clang test suite for AST serialization because it just hasn't been completed yet. I have a feeling that changing the behavior of GetCurrentBitNo() will break things.<br> <br> I'm actually a little surprised that GetCurrentBitNo() needs to be changed. While the AST serialization work is incomplete, it has worked for non-trivial cases. Since it uses GetCurrentBitNo() to jump around the bitcode file, I'm suspicious that anything needs to be changed.<br> <br> Also, what happens if NextChar == FirstChar? Wouldn't (NextChar-FirstChar-4)*8 be a negative number?<div></div></blockquote><div><br>It will be negative. But in that case, BitsInCurWord will be 0, the final result is 0.<br> </div></div></blockquote></div><br></div></div><div>Okay. If GetCurrentBitNo() is broken then it should be fixed. I personally haven't looked at it in detail recently, so I'm not certain what the ramifications would be if we changed it or why it seems to work correctly now (or maybe it doesn't?).</div> </div></blockquote></div><br>I do think GetCurrentBitNo() is broken, because it is not returning the bit # we are reading. But it works now, partially because we have code like this:<br><br> if (WordBitNo) {<br>- NextChar -= 4;<br> Read(static_cast<unsigned>(WordBitNo));<br><br>I don't know if there is other code doing the similar compensate work.<br></blockquote></div><div><br></div><div>Hmm. This is definitely worth investigating further. At the very least there should be more comments documenting the invariants/assumptions in the code with regards to the bit location.</div></body></html>