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). > > I haven't been able to isolate the bug in a super-simple test case, but I > can reproduce it in less than 100 lines in a single translation unit. > > The bug is triggered by logically incorrect, but I believe well-defined code. > I will explain as well as I can. > > First the compiler version: > > g++ (GCC) 4.7.0 20110829 (experimental) > > The compiler came from downloading and unzipping TMRWB: > > x86_64-w64-mingw32-gcc-4.7.0-stdthread_rubenvb.7z > > I believe that the following test code is correct when compiled using a > 32-bit compiler. (It compiles and runs correctly using the following > 32-bit mingw32 build: g++ (TDM-2 mingw32) 4.4.1.) When run, it gives > the following output: > > C:\>stdout_stream_error > hello... > xsputn: top of loop, pos = 0 > Message 1 (with '\n')... > xsputn: bottom of loop, pos = 24 > xsputn: top of loop, pos = 24 > xsputn: bottom of loop, pos = -1 > xsputn: before return, n = 25, pos = -1 > xsputn: top of loop, pos = 0 > Message 2 (with '\n')... > xsputn: bottom of loop, pos = 24 > xsputn: top of loop, pos = 24 > xsputn: bottom of loop, pos = -1 > xsputn: before return, n = 25, pos = -1 > goodbye! > > I believe that the code is logically incorrect when compiled using a > 64-bit compiler, and should enter an infinite loop. Instead, I get the > following output, which is unexpectedly wrong as I will explain further. > > C:\>stdout_stream_error > hello... > xsputn: top of loop, pos = 0 > Message 1 (with '\n')... > xsputn: bottom of loop, pos = 24 > xsputn: top of loop, pos = 24 > goodbye! > > 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; > } > > 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.) > > Line 47 is "return n;", the return statement of xsputn. > > Specifically, setting a breakpoint at the return statement (line 47) > seems to actually set the breakpoint at the next executable line of > code, which is line 56. "int main (int argc, char *argv[]) {" > > Here's a gdb session that shows this: > > C:\>gdb stdout_stream_error > GNU gdb (GDB) 7.3.0.20110829-cvs > This GDB was configured as "x86_64-w64-mingw32". > <http://www.gnu.org/software/gdb/bugs/>... > Reading symbols from C:/stdout_stream_error.exe...done. > (gdb) b stdout_stream_error.cpp:47 > Breakpoint 1 at 0x401680: file stdout_stream_error.cpp, line 47. > (gdb) run > Starting program: C:/stdout_stream_error.exe > [New Thread 9196.0x1904] > > Breakpoint 1, main (argc=1, argv=0x409008) at stdout_stream_error.cpp:56 > 56 int main (int argc, char *argv[]) { > (gdb) cont > Continuing. > hello... > xsputn: top of loop, pos = 0 > Message 1 (with '\n')... > xsputn: bottom of loop, pos = 24 > xsputn: top of loop, pos = 24 > goodbye! > [Inferior 1 (process 9196) exited normally] > (gdb) > > Note that execution hits a breakpoint when entering main, but (after > continuing) does not hit a breakpoint when returning from xsputn. > > Some further comments: > > I've tried to reproduce this bug in a setting without the basic_streambuf > stuff and have not been able to. For example, here is a test program that > does not reproduce the bug: > > ==================== > > compiled as: g++ -g -o npos_test npos_test.cpp > > ==================== > > // try to reproduce std::string::npos loop bug > > #include <iostream> > #include <string> > > #include <cstdio> > > size_t dec_pos (unsigned pos) { > size_t r = --pos; > if (r == 0) r = std::string::npos; > return r; > } > > int main (int argc, char *argv[]) { > > printf ("hello...\n"); > > unsigned pos = 3; > // size_t pos = 3; > while (pos != std::string::npos) { > // while (pos != ((unsigned) -1)) { > printf ("top of loop, pos = %d\n", pos); > // pos--; > // if (pos == 0) pos = std::string::npos; > pos = dec_pos (pos); > if (pos != std::string::npos) { > // if (pos != ((unsigned) -1)) { > printf ("in if, pos = %d\n", pos); > } > printf ("bottom of loop, pos = %d\n", pos); > } > > printf ("goodbye!\n"); > > return 0; > } > > ==================== > > When compiled with the 64-bit compiler in its logically incorrect form > (with pos declared as "unsigned pos;") it enters an infinite loop, as > expected. When either compiled with the 32-bit compiler or when patched > so that pos is declared correctly ("size_t pos;"), it runs as expected. > > I've tried a number of different things to further isolate this bug, so > there is a lot more detail I could give, but I'm not sure it would be > enlightening. (But ask if you want.) > > I can also explain why I think this is a real bug, and not incorrect code > invoking "undefined behavior." > > Lastly, for context, I came across this issue when upgrading a little Qt > utility class (QDebugStream) that someone had posted on a Qt list from > the 32-bit to the 64-bit compiler. It's not really relevant, but that > discussion can be found at: > > http://lists.qt.nokia.com/pipermail/qt-interest/2011-October/036651.html > http://lists.qt.nokia.com/pipermail/qt-interest/2011-November/036770.html > > > 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. If you are using here instead of 'unsigned' a 'signed' n, everything works as expected. 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
