2011/11/5 K. Frank <[email protected]>: > Hi Kai! > > Thanks for your response. > > I don't follow what you are saying -- please see my comments at > the bottom. > > On Sat, Nov 5, 2011 at 4:29 AM, Kai Tietz <[email protected]> wrote: >> 2011/11/5 K. Frank <[email protected]>: >>> Hello List (and Ruben)! >>> >>> I think I have come across a bug in Ruben's 64-bit 4.7.0 build, otherwise >>> known as "The Mighty Ruben's Wacky Build" ("TMRWB") (TM). >>> ... >>> The bug is triggered by logically incorrect, but I believe well-defined >>> code. >>> I will explain as well as I can. >>> ... >>> Here is the code for the test program: >>> >>> ==================== >>> >>> compiled as: g++ -g -o stdout_stream_error stdout_stream_error.cpp >>> >>> ==================== >>> >>> #include <iostream> >>> #include <streambuf> >>> #include <string> >>> >>> #include <cstdio> >>> >>> class StdoutStream : public std::basic_streambuf<char> >>> { >>> public: >>> StdoutStream(std::ostream &stream) : m_stream (stream) { >>> m_old_buf = stream.rdbuf(); >>> stream.rdbuf (this); >>> } >>> ~StdoutStream() { >>> // output anything that is left >>> if (!m_string.empty()) printf ("%s", m_string.c_str()); >>> m_stream.rdbuf (m_old_buf); >>> } >>> >>> protected: >>> virtual int_type overflow(int_type v) { >>> if (v == '\n') { >>> printf ("%s", m_string.c_str()); >>> m_string.erase (m_string.begin(), m_string.end()); >>> } >>> else m_string += v; >>> return v; >>> } >>> > > ***** The only use of std::streamsize is here ***** > ***** (and in the return statement). ***** > >>> virtual std::streamsize xsputn (const char *p, std::streamsize n) { >>> m_string.append (p, p + n); >>> unsigned pos = 0; >>> // size_t pos = 0; // this is the correct declaration >>> while (pos != std::string::npos) { // 32-bit unsigned compared >>> against 64-bit size_t >>> // while (pos != ((unsigned) -1)) { // this works >>> printf ("xsputn: top of loop, pos = %d\n", pos); >>> pos = m_string.find ('\n'); >>> if (pos != std::string::npos) { // 32-bit unsigned compared >>> against 64-bit size_t >>> // if (pos != ((unsigned) -1)) { // this works >>> std::string tmp(m_string.begin(), m_string.begin() + pos + 1); >>> printf ("%s", tmp.c_str()); >>> m_string.erase (m_string.begin(), m_string.begin() + pos + 1); >>> } >>> printf ("xsputn: bottom of loop, pos = %d\n", pos); >>> } >>> printf ("xsputn: before return, n = %d, pos = %d\n", n, pos); >>> return n; >>> } >>> >>> private: >>> std::ostream &m_stream; >>> std::streambuf *m_old_buf; >>> std::string m_string; >>> }; >>> >>> int main (int argc, char *argv[]) { >>> >>> printf ("hello...\n"); >>> >>> StdoutStream *out = new StdoutStream (std::cout); >>> >>> if (out) { // to avoid unused variable warning >>> // prints neither message to text1 >>> // std::cout << "Message 1 (with endl)..." << std::endl; >>> // std::cout << "Message 2 (with endl)..." << std::endl; >>> >>> // prints only "Message 1" to text1 >>> try { >>> std::cout << "Message 1 (with '\\n')...\n"; >>> std::cout << "Message 2 (with '\\n')...\n"; >>> } >>> catch (...) { // no exception thrown -- not the ptoblem >>> printf ("exception caught...\n"); >>> } >>> >>> // prints both messages to text1 >>> // std::cout << "Message 1 (with two '\\n's)...\nMessage 2 (with >>> two '\\n's)...\n"; >>> } >>> >>> printf ("goodbye!\n"); >>> >>> return 0; >>> } >>> >>> ==================== >>> >>> The code is, I believe, logically incorrect in the member function xsputn() >>> in that the local variable "pos" should be declared as >>> >>> size_t pos; >>> >>> rather than >>> >>> unsigned pos; >>> >>> This is because (under the 64-bit compiler) unsigned is 32 bits, while >>> size_t is 64 bits. >>> >>> However, I would expect this error to cause the loop never to terminate. >>> Instead, the function exits, but according to gdb, not through its return >>> statement! >>> >>> In fact, it seems to me that according to gdb, the return statement has >>> been somehow optimized away. (This is just speculation on my part.) >>> ... >>> I can also explain why I think this is a real bug, and not incorrect code >>> invoking "undefined behavior." > > A quick comment on this. Kai raises the issue of signed / unsigned > conversions (because std::streamsize is signed). I've raised the issue > of narrower (32-bit) / wider (64-bit) unsigned conversions. As I explain > below, I don't think std::streamsize enters into this, but even if it did, I > still believe that the compiler is not behaving properly. > > As I read the standard, neither the signed / unsigned nor the narrower / > wider conversions invoke undefined behavior. Narrower / wider integral > are, I believe, well specified by the standard. I believe that signed / > unsigned integral conversions can be implementation-defined (based > on how the implementation / hardware represents negative integers, > and how it implements sign extension), but not undefined. > > No matter how the conversions are implemented, as I look at the code, > I think that the behavior should be well defined and the function should > either enter an infinite loop or run correctly. Exiting the loop apparently > early (no matter what the conversions are) seems to me to be a compiler > bug. > >>> ... >>> Thanks for anyone's thoughts. >>> >>> K. Frank >> >> Hmm, this looks to me indeed as logical wrong code, but by different >> reason as you are assuming. The underlying issue here is that >> streamsize is derived from ptrdiff_t, which is a signed type. But you >> are using here an 'unsigned'. >> The expanding rule from unsigned to signed with higher precision >> remains unsigned valued. For 32-bit we have here for both types same >> precision, and therefore indeed the sign check is done as intended. > > Here's what I don't understand about your comment: In the code above, > std::streamsize is used explicitly in two places: > > virtual std::streamsize xsputn (const char *p, std::streamsize n) { > > as the return type of xsputn, and as the type of the argument n. It is > also used implicitly, if you will, in the return statement of xsputn: > > return n; > > But n is never modified in the code, and is only used, as I understand it, > in a fully legal pointer-arithmetic expression: > > m_string.append (p, p + n); > > (p is a char*). > > So I just don't see any signed / unsigned mismatch between std::streamsize > and any other integral type. > >> If you are using here instead of 'unsigned' a 'signed' n, everything >> works as expected. > > I don't follow what the concrete change to the code would be. I'm happy > to make such a change and see if it fixes the problem, but I don't see > what specific change you are proposing. > > If you mean changing the type of the argument n to be unsigned: > > virtual std::streamsize xsputn (const char *p, unsigned n) { > > I don't think that I would be allowed to do that because the signature of > xsputn is defined by the iostream library (and hence by the standard). > > (Of course, when I say I think there is an error in the compiler, I mean > the whole compiler / library package. The compiler could be okay, but > there could be an error in the library code. To clarify, I do believe that > the compiler is generating incorrect code, but if a coding error in the > library were invoking undefined behavior, the compiler would technically > be within its rights to do so.) > >> Regards, >> Kai > > Further thoughts would be greatly appreciated.
See, if I have a 32-bit unsigned scalar integer and I convert it into a 64-bit signed integer conversion, it is wrong believe that a sign-conversion is done here. The unsigned 32-bit integer scalar fits into value-range of the 64-bit signed integer scalar. So result of widen cast remains unsigned. As by rule, always for a comparison of X == Y - where X has 32-bit width, and Y has 64-bit width - value gets promoted to wider-scalar width for comparision, we get into your described behavior. so the comparsion X == -1LL can be never true. And this is what you are actually try do here. As X gets promoted for value 0xfffffffff to 0xffffffffLL and not as you trying to suggest to 0xffffffffffffffffLL. Regards, Kai ------------------------------------------------------------------------------ RSA(R) Conference 2012 Save $700 by Nov 18 Register now http://p.sf.net/sfu/rsa-sfdev2dev1 _______________________________________________ Mingw-w64-public mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
