boost serialization crash with clang 5.0.0

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

boost serialization crash with clang 5.0.0

Eric Fiselier via cfe-dev
Hi,

I tried to upgrade to clang 5.0.0 and found that a program that uses
the boost serialization library crashes with a null pointer
dereference during serialization.

The relevant part of boost serialization (from
https://github.com/boostorg/serialization/blob/develop/include/boost/archive/detail/oserializer.hpp)
is:

    template<class T>
    static const basic_pointer_oserializer * register_type(Archive
&ar, T & /*t*/){
        // there should never be any need to save an abstract polymorphic
        // class pointer.  Inhibiting code generation for this
        // permits abstract base classes to be used - note: exception
        // virtual serialize functions used for plug-ins
        typedef
            typename mpl::eval_if<
                boost::serialization::is_abstract< T >,
                mpl::identity<abstract>,
                mpl::identity<non_abstract>
            >::type typex;
        return typex::template register_type< T >(ar);
    }

    template<class TPtr>
    static void invoke(Archive &ar, const TPtr t){
        register_type(ar, * t);
        if(NULL == t){
            basic_oarchive & boa
                =
boost::serialization::smart_cast_reference<basic_oarchive &>(ar);
            boa.save_null_pointer();
            save_access::end_preamble(ar);
            return;
        }
        save(ar, * t);
    }

Clang 5.0.0 removes the check for a null pointer.
GCC 7.2 and Clang 4.0.1 keep the check

I see that t is dereferenced before the null check, but memory isn't
actually accessed until afterwards.

Simplified reproduction: https://godbolt.org/g/L7zC82

Is this a bug in clang 5.0.0, or in boost serialization?

--
Malcolm Parsons
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: boost serialization crash with clang 5.0.0

Eric Fiselier via cfe-dev
[1] and [2] seems to be related to your case. I try add
-fcatch-undefined-behavior on https://godbolt.org/g/L7zC82 , and get
compilation failure. If the code does have undefined behavior, the
code should be fixed.

[1] http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html
[2] https://blog.regehr.org/archives/970

Regards,
chenwj


2017-09-22 19:00 GMT+08:00 Malcolm Parsons via cfe-dev <[hidden email]>:

> Hi,
>
> I tried to upgrade to clang 5.0.0 and found that a program that uses
> the boost serialization library crashes with a null pointer
> dereference during serialization.
>
> The relevant part of boost serialization (from
> https://github.com/boostorg/serialization/blob/develop/include/boost/archive/detail/oserializer.hpp)
> is:
>
>     template<class T>
>     static const basic_pointer_oserializer * register_type(Archive
> &ar, T & /*t*/){
>         // there should never be any need to save an abstract polymorphic
>         // class pointer.  Inhibiting code generation for this
>         // permits abstract base classes to be used - note: exception
>         // virtual serialize functions used for plug-ins
>         typedef
>             typename mpl::eval_if<
>                 boost::serialization::is_abstract< T >,
>                 mpl::identity<abstract>,
>                 mpl::identity<non_abstract>
>             >::type typex;
>         return typex::template register_type< T >(ar);
>     }
>
>     template<class TPtr>
>     static void invoke(Archive &ar, const TPtr t){
>         register_type(ar, * t);
>         if(NULL == t){
>             basic_oarchive & boa
>                 =
> boost::serialization::smart_cast_reference<basic_oarchive &>(ar);
>             boa.save_null_pointer();
>             save_access::end_preamble(ar);
>             return;
>         }
>         save(ar, * t);
>     }
>
> Clang 5.0.0 removes the check for a null pointer.
> GCC 7.2 and Clang 4.0.1 keep the check
>
> I see that t is dereferenced before the null check, but memory isn't
> actually accessed until afterwards.
>
> Simplified reproduction: https://godbolt.org/g/L7zC82
>
> Is this a bug in clang 5.0.0, or in boost serialization?
>
> --
> Malcolm Parsons
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev



--
Wei-Ren Chen (陳韋任)
Homepage: https://people.cs.nctu.edu.tw/~chenwj
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: boost serialization crash with clang 5.0.0

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
Hi Malcolm,

From my perspective, it seems to be an undefined behaviour anyway, so a bug in Boost.Serialization.
At least, that's how I interpret [dcl.ref] "References" (11.3.2 in latest draft), paragraph 5:

[ Note: In particular, a null reference cannot exist in a well-defined program, because the only way to create such a reference would be to bind it to the “object” obtained by indirection through a null pointer, which causes undefined behavior. ... ]

Here, the null reference created from `*t` may technically exist in `register_type`, even if it's not used, so I'd argue that clang's 5 conclusion about UB is correct. Even more, that's already the "indirection through a null pointer" that causes UB.

On the other hand, decltype is a pure-type manipulation, therefore there is no possible UB.

A small fix does the job: https://godbolt.org/g/mSLgqr.
Using `decltype(*t)` instead of passing `*t` as argument does not provoke the same behaviour.

The relevant snippet with changes:

template<class T, class U = typename remove_reference<T>::type>
static void register_type(Archive &ar){
non_abstract::template register_type<U>(ar);
}

template<class TPtr>
static void invoke(Archive &ar, const TPtr t){
register_type<decltype(*t)>(ar);
if(NULL == t) {
ar.save_null_pointer();
return;
}
save(ar, * t);
}

Best regards,
Marek Kurdej 

From: Malcolm Parsons via cfe-dev <[hidden email]>
To: cfe-dev <[hidden email]>
Cc: [hidden email]
Bcc: 
Date: Fri, 22 Sep 2017 12:00:26 +0100
Subject: [cfe-dev] boost serialization crash with clang 5.0.0
Hi,

I tried to upgrade to clang 5.0.0 and found that a program that uses
the boost serialization library crashes with a null pointer
dereference during serialization.
--
Marek

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev