Modify

Opened 2 years ago

Closed 8 weeks ago

#11632 closed Bugs (fixed)

UB in boost.format basic_oaltstringstream

Reported by: rogero@… Owned by: James E. King, III
Milestone: Boost 1.66.0 Component: format
Version: Boost 1.57.0 Severity: Problem
Keywords: ubsan Cc:

Description

The constructors for basic_oaltstringstream in boost/format/alt_sstream.hpp pass the result of a member call to the constructor of a base class.

For example the default constructor is

basic_oaltstringstream() : pbase_type(new stringbuf_t), stream_t(rdbuf()) {}

Where stream_t is a typedef for the second base class.

I discovered this when using ubsan and I initially raised GCC pr67515 using a much simplified test-case; but as Jon Wakely pointed out: "12.6.2 [class.base.init] p16 says this is undefined (and has a very similar example)."

One solution to avoid UB in this case is to manually inline the call to rdbuf() and so initialise the member of the second base class directly rather than via the rdbuf() function; so for example change

pbase_type(new stringbuf_t), stream_t(rdbuf())

to

pbase_type(new stringbuf_t), stream_t(pbase_type::member.get())

in the default ctor (and similarly for the other constructors.)

Attachments (0)

Change History (16)

comment:1 Changed 2 years ago by anonymous

Note that the suggested resolution assumes the anticipated direction of CWG 2056.

comment:2 Changed 2 years ago by viboes

Component: Noneformat
Owner: set to Samuel Krempp

comment:3 Changed 2 years ago by anonymous

Proposed change (rdbuf() -> pbase_type::member.get() in constructors) fixes the undefined behavior for me on GCC 5.3.1, thanks!

comment:4 Changed 2 years ago by anonymous

Hmmm... I've applied the following patch:

diff -Naur alt_sstream.hpp.orig alt_sstream.hpp 
--- alt_sstream.hpp.orig	2015-12-11 11:21:50.000000000 +0100
+++ alt_sstream.hpp	2016-01-18 10:46:39.000000000 +0100
@@ -137,13 +137,13 @@
         public:
             typedef Alloc  allocator_type;
             basic_oaltstringstream() 
-                : pbase_type(new stringbuf_t), stream_t(rdbuf()) 
+                : pbase_type(new stringbuf_t), stream_t(pbase_type::member.get()) 
                 { }
             basic_oaltstringstream(::boost::shared_ptr<stringbuf_t> buf) 
-                : pbase_type(buf), stream_t(rdbuf()) 
+                : pbase_type(buf), stream_t(pbase_type::member.get()) 
                 { }
             basic_oaltstringstream(stringbuf_t * buf) 
-                : pbase_type(buf, No_Op() ), stream_t(rdbuf()) 
+                : pbase_type(buf, No_Op() ), stream_t(pbase_type::member.get()) 
                 { }
             stringbuf_t * rdbuf() const 
                 { return pbase_type::member.get(); }

The first instance of undefined behavior was indeed no longer there, but now I get a different, slightly modified backtrace:

ASAN:SIGSEGV
=================================================================
==20533==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f28b5b56110 bp 0x7ffda52eb000 sp 0x7ffda52eab50 T0)
    #0 0x7f28b5b5610f in __dynamic_cast (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x6610f)
    #1 0x7f28b4deabcf  (/usr/lib/x86_64-linux-gnu/libubsan.so.0+0x9bcf)
    #2 0x7f28b4dea132  (/usr/lib/x86_64-linux-gnu/libubsan.so.0+0x9132)
    #3 0x7f28b4dea892 in __ubsan_handle_dynamic_type_cache_miss (/usr/lib/x86_64-linux-gnu/libubsan.so.0+0x9892)
    #4 0x7f28b9b1759b in boost_1_57_0::io::basic_oaltstringstream<char, std::char_traits<char>, std::allocator<char> >::basic_oaltstringstream(boost_1_57_0::io::basic_altstringbuf<char, std::char_traits<char>, std::allocator<char> >*) (/build/debug/cpp/libotdscpp.so+0x3d1a59b)
    #5 0x7f28b9b0cc0a in void boost_1_57_0::io::detail::put<char, std::char_traits<char>, std::allocator<char>, boost_1_57_0::io::detail::put_holder<char, std::char_traits<char> > const&>(boost_1_57_0::io::detail::put_holder<char, std::char_traits<char> > const&, boost_1_57_0::io::detail::format_item<char, std::char_traits<char>, std::allocator<char> > const&, boost_1_57_0::basic_format<char, std::char_traits<char>, std::allocator<char> >::string_type&, boost_1_57_0::basic_format<char, std::char_traits<char>, std::allocator<char> >::internal_streambuf_t&, std::locale*) (/build/debug/cpp/libotdscpp.so+0x3d0fc0a)
    #6 0x7f28b9aff54f in void boost_1_57_0::io::detail::distribute<char, std::char_traits<char>, std::allocator<char>, boost_1_57_0::io::detail::put_holder<char, std::char_traits<char> > const&>(boost_1_57_0::basic_format<char, std::char_traits<char>, std::allocator<char> >&, boost_1_57_0::io::detail::put_holder<char, std::char_traits<char> > const&) lib/boost/install/include/boost/format/feed_args.hpp:285
    #7 0x7f28b9af37c0 in boost_1_57_0::basic_format<char, std::char_traits<char>, std::allocator<char> >& boost_1_57_0::io::detail::feed_impl<char, std::char_traits<char>, std::allocator<char>, boost_1_57_0::io::detail::put_holder<char, std::char_traits<char> > const&>(boost_1_57_0::basic_format<char, std::char_traits<char>, std::allocator<char> >&, boost_1_57_0::io::detail::put_holder<char, std::char_traits<char> > const&) lib/boost/install/include/boost/format/feed_args.hpp:295
    #8 0x7f28ba15ed60 in boost_1_57_0::basic_format<char, std::char_traits<char>, std::allocator<char> >& boost_1_57_0::io::detail::feed<char, std::char_traits<char>, std::allocator<char>, char const (&) [27]>(boost_1_57_0::basic_format<char, std::char_traits<char>, std::allocator<char> >&, char const (&) [27]) lib/boost/install/include/boost/format/feed_args.hpp:307
    #9 0x7f28ba159694 in boost_1_57_0::basic_format<char, std::char_traits<char>, std::allocator<char> >& boost_1_57_0::basic_format<char, std::char_traits<char>, std::allocator<char> >::operator%<char [27]>(char const (&) [27]) lib/boost/install/include/boost/format/format_class.hpp:64
$ g++-5 -v
Using built-in specs.
COLLECT_GCC=/usr/bin/g++-5
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/5/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 5.3.0-3ubuntu1~14.04' --with-bugurl=file:///usr/share/doc/gcc-5/README.Bugs --enable-languages=c,ada,c++,java,go,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-5 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=gcc4-compatible --disable-libstdcxx-dual-abi --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-5-amd64/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-5-amd64 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-5-amd64 --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --disable-werror --with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 5.3.0 20151204 (Ubuntu 5.3.0-3ubuntu1~14.04) 

Any ideas?

comment:5 Changed 2 years ago by anonymous

Sadly no; the fix I submitted worked for our (limited) use of boost::format with ubsan. Roger.

comment:6 Changed 23 months ago by anonymous

Okay, an update on this. I have done a clean rebuild, and the attached patch actually does seem to work for me as well as I have initially indicated. I'm sorry for the misleading comment. Apparently our build system didn't actually pick up my changes and I've got confused by the old error message. Once again, sorry about this...

In the light of this additional information, it would be great if this patch could be applied to have the problem fixed out of the box.

comment:7 Changed 23 months ago by rogero@…

That's good news, and I agree it would be good if this patch were to be applied.

comment:8 Changed 23 months ago by Georg Sauthoff <mail@…>

I can confirm this issue on Fedora 23 (Boost 1.58, gcc 5.1). It is exposed when using the ubsanitizer and boost::format().

With Fedora 21 (gcc 4.9 and upstream Boost 1.58) the usage of boost::format() didn't trigger this issue.

comment:9 Changed 16 months ago by Eli Fidler <efidler@…>

I can further confirm that this fixes the UBSan issues on GCC 5.4 and AppleClang? 8. It's been 7 months; is there any progress toward getting this landed?

comment:10 Changed 13 months ago by anonymous

Can also confirm that this fixes UBSAN issues with GCC 5.3 and Boost 1.57.0.

comment:11 Changed 6 months ago by anonymous

any idea when this fix will be applied?

comment:12 Changed 2 months ago by James E. King, III

Keywords: ubsan added
Milestone: To Be DeterminedBoost 1.66.0
Owner: changed from Samuel Krempp to James E. King, III
Version: Boost Development TrunkBoost 1.57.0

comment:13 Changed 2 months ago by James E. King, III

Submitted a pull request in github for this, as I ran into it on one of my projects: https://github.com/boostorg/format/pull/13

comment:14 Changed 2 months ago by James E. King, III

Resolution: fixed
Status: newclosed

comment:15 Changed 2 months ago by James E. King, III

Resolution: fixed
Status: closedreopened

Reopening this to track that it has been fixed in develop but not yet merged into master.

comment:16 Changed 8 weeks ago by James E. King, III

Resolution: fixed
Status: reopenedclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain James E. King, III.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.