change Revision: 81616 Author: marshall Date: 11:12:42 AM, Wednesday, November 28, 2012 Message: Fixed incorrect 'const' param for utf8_codecvt_facet::do_length ---- Modified : /trunk/boost/detail/utf8_codecvt_facet.hpp This change causes the a failure in the serialization library test suite with "test_utf8_codecvt" on all platforms which use the gcc standard library. I'm not sure what motivated the change or what it addressed but I'm pretty sure it's causing this (and a couple other) test failures. Note that all the failures are related to usage of wchar_t - perhaps that helps. Note that the test is part of the serialization library test suite - but it has nothing to do with serialization per se and doesn't use any of the serialization library. It just happens to be there because there was no more convenient place to put it. Marshall - could you please look into this? Robert Ramey
On Dec 10, 2012, at 9:27 PM, Robert Ramey <ramey@rrsd.com> wrote:
Ok, I'm seeing that too - the test doesn't "fail" :-(, it just hangs.
Back on June 28th, I posted a message to the list titled "[Survey] utf8_codecvt_facet::do_length parameter" which described the problem:
FWIW, I got zero responses. Then Microsoft shipped Visual Studio 12, which also defined the parameter as "not const". I looked at LibComo, and it too defines the parameter as non-const. So, after discussing this with Beman (since it's his library), I changed the code to look like this: // Only do CONST on older Dinkumware libraries. #if defined(_CPPLIB_VER) && (_CPPLIB_VER < 540) #define BOOST_CODECVT_DO_LENGTH_CONST const #else #define BOOST_CODECVT_DO_LENGTH_CONST #endif If that's causing problems for you, I apologize, and I'll be happy to work with you to fix it - though I don't understand _how_ it's causing problems. I tried changing the macro to always be "const", and the test still hung for me (on both clang and gcc) Looking into this a bit more, it appears that Boost.IOStreams has it's own #define for this. Sigh. -- Marshall Marshall Clow Idio Software <mailto:mclow.lists@gmail.com> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
Marshall Clow wrote: ... No good deed goes unpunished.
I'm at a loss here as well. I can't see why such a change would cause the test to fail with wchar_t types. For all I know, the test might be wrong. But then a couple (not all) serialization library tests fail - all these use codecvt with wchar_t. The failure indicates that there is some problem with malloc usage like double delete. So that would be pretty deep in some component. The fact that it doesn't fail every where suggests that there is some memory overwrite which sometimes corrupts the heap which then shows up as this test failure. I've had trouble setting up GDB for this test. This would be my normal approach. Robert Ramey
On Dec 11, 2012, at 9:09 AM, Robert Ramey <ramey@rrsd.com> wrote:
I've had trouble setting up GDB for this test. This would be my normal approach.
Ok - here's what I've found: Here's the crash: (mac os X, gcc library 4.2.1):
You can see that it's destructing a std::basic_ifstream. If I strip a bunch of stuff out of test_array.cpp, the crash doesn't happen.
If I include the line:
test_iarchive ia(is, TEST_ARCHIVE_FLAGS);
then it does. This suggests to me that something in the test_iarchive is poking at the test_istream - but I don't know what. I tried to extract this into a simple test case (attached), but I can't figure out what to link against for the wide case. clang++ junk.cpp -DNARROW -I /Sources/boost/trunk -L $BUILD/libs/serialization/$TAIL -lboost_serialization # works clang++ junk.cpp -I /Sources/boost/trunk -L $BUILD/libs/serialization/$TAIL -lboost_wserialization # does not For the narrow case, I link against lib boost_serialization, and everything is good. For the wide case, when I link against boost_wserialization, I get the following errors:
Can you tell me how to solve this? -- Marshall Marshall Clow Idio Software <mailto:mclow.lists@gmail.com> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
Marshall Clow wrote:
for the wide case, one would have to link with BOTH boost_serialization as well as boost_wserialization. As I stated before, I don't think it's a good idea to try to fix this at this time. a) The approach above "might" fix the issue with that one test. But there are 50 other tests with the same archive which pass. I'm suspected that it's an issue with this particular test which interrupts the the process. That is its some sort of exception safety issue. b) Actually, I don't think it's that. I think it's a conflict between the gcc runtime library and our codecvt facet which reimplements the same functionaly with "almost" the same interface. c) In other words, our failed test is not the bug - but a symptom of a more subtle bug. If I just make the test past, I'll be effectively hiding the bug rather than fixing it. I think it needs a real fix: a) tweak the headers so that it used the built-in codecvt facets where available. b) Enhance the codecvt test (or add a couple more) to verify: i) that the built in codevt facet actually works as expected. ii) that our utf8_codecvt facet interoperates with the built in one. That is that a file created with one can be read by the other and vice-versa. The change made in the trunk addresses an issue which has been in the code for years and only produces a warning. I would prefer that we take our time and address it in a more definitive manner. To try to fix it on short notice at this point isn't worth the risk of imposing a setback on the release. Robert Ramey
On Jan 5, 2013, at 10:53 PM, Robert Ramey <ramey@rrsd.com> wrote:
Linking with boost_wserialization as well as boost_serialization gives fewer linker errors, but still some:
-- Marshall Marshall Clow Idio Software <mailto:mclow.lists@gmail.com> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
On Jan 5, 2013, at 10:53 PM, Robert Ramey <ramey@rrsd.com> wrote:
The change made in the trunk addresses an issue which has been
in the code for years and only produces a warning.
For people who build with "warning as errors', this is a showstopper.
This is not "short notice"; I've been trying to get this bug fixed for over a year. I first proposed fixing this for 1.49. This is now an issue for Visual Studio users as well, since they have changed their codecvt to be standards compliant in VS 2012. Boost is now in a position where we have a slower reaction time than Microsoft on this issue. -- Marshall Marshall Clow Idio Software <mailto:mclow.lists@gmail.com> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
Marshall Clow wrote:
I'm not disputing any of the above. I'm just not convinced that this way of addressing the issue will actually address the source of the problem which I believe is more subtle than it might first appear. Right now we have a simple choice - merge in the change which eliminates the warning but trips some error on one of the tests of the serialization library or don't do it which leaves the warning and permits the serialization library test to pass. I'm actually indifferent to which decision everyone wants to make.
From my view either option leaves the utf8_codecvt where it has some unknown behavior.
I DO think it's a bad idea just to tweak the serialization library test just so it passes the test. I would prefer to consider the warning at face value. This warning indicates that we have named a function in utf8_codecvt which is also implemented in the base class. This in turn means we should actually look at the base class header and determine what is really going on. Given the fact that there is a lot of variation among libraries - this requires some time and effort. Robert Ramey
note another issue here. Originally the utf8_codecvt_facet was sort of it's own library. This made a lot of sense since it is used by at least three other libraries: serialization, file_system, and program options. But strident objects were raised about this since it hadn't been reviewed. so it was stuffed into a detail header. But this left a problem as to where to put a test and documentation page for it. Also the question arose as to where to put the source code for it. The library was made as a template and instatiated by each library which uses it. I required at least minimal testing and documentation so I put this stuff into a section of the serialization library. Not a good situation - a lot messier than it has to be. This needs to be a "real" library with it's independent documentation and tests. Robert Ramey
On Jan 5, 2013, at 10:53 PM, Robert Ramey <ramey@rrsd.com> wrote:
Here's my dirt-simple test case: #include <fstream> #include <boost/config.hpp> #include <boost/archive/binary_wiarchive.hpp> int main(int argc, char *argv [] ) { std::wifstream is(argv[1], std::wios::binary); try { boost::archive::binary_wiarchive ia(is, 0); } catch (boost::archive::archive_exception ae){} return 0; } When I build it thus: g++ ser-test2.cpp -I /Sources/boost/trunk -lboost_wserialization -lboost_serialization I get: Undefined symbols for architecture x86_64: "boost::archive::basic_binary_iarchive<boost::archive::binary_wiarchive>::load_override(boost::archive::class_name_type&, int)", referenced from: void boost::archive::binary_iarchive_impl<boost::archive::binary_wiarchive, wchar_t, std::char_traits<wchar_t> >::load_override<boost::archive::class_name_type>(boost::archive::class_name_type&, int)in ccfHj5hR.o "boost::archive::basic_binary_iarchive<boost::archive::binary_wiarchive>::init()", referenced from: boost::archive::binary_iarchive_impl<boost::archive::binary_wiarchive, wchar_t, std::char_traits<wchar_t> >::init(unsigned int)in ccfHj5hR.o "boost::archive::basic_binary_iprimitive<boost::archive::binary_wiarchive, wchar_t, std::char_traits<wchar_t> >::init()", referenced from: boost::archive::binary_iarchive_impl<boost::archive::binary_wiarchive, wchar_t, std::char_traits<wchar_t> >::init(unsigned int)in ccfHj5hR.o "boost::archive::basic_binary_iprimitive<boost::archive::binary_wiarchive, wchar_t, std::char_traits<wchar_t> >::basic_binary_iprimitive(std::basic_streambuf<wchar_t, std::char_traits<wchar_t> >&, bool)", referenced from: boost::archive::binary_iarchive_impl<boost::archive::binary_wiarchive, wchar_t, std::char_traits<wchar_t> >::binary_iarchive_impl(std::basic_istream<wchar_t, std::char_traits<wchar_t> >&, unsigned int)in ccfHj5hR.o "boost::archive::basic_binary_iprimitive<boost::archive::binary_wiarchive, wchar_t, std::char_traits<wchar_t> >::~basic_binary_iprimitive()", referenced from: boost::archive::binary_iarchive_impl<boost::archive::binary_wiarchive, wchar_t, std::char_traits<wchar_t> >::~binary_iarchive_impl()in ccfHj5hR.o boost::archive::binary_iarchive_impl<boost::archive::binary_wiarchive, wchar_t, std::char_traits<wchar_t> >::~binary_iarchive_impl()in ccfHj5hR.o boost::archive::binary_iarchive_impl<boost::archive::binary_wiarchive, wchar_t, std::char_traits<wchar_t> >::~binary_iarchive_impl()in ccfHj5hR.o boost::archive::binary_iarchive_impl<boost::archive::binary_wiarchive, wchar_t, std::char_traits<wchar_t> >::binary_iarchive_impl(std::basic_istream<wchar_t, std::char_traits<wchar_t> >&, unsigned int)in ccfHj5hR.o ld: symbol(s) not found for architecture x86_64 collect2: ld returned 1 exit status What am I missing? -- Marshall Marshall Clow Idio Software <mailto:mclow.lists@gmail.com> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
participants (2)
-
Marshall Clow -
Robert Ramey