Opened 5 years ago

Last modified 5 years ago

#13481 new Bugs

Strict aliasing is causing SIGSEGV on ARM Cortex-A15 when using GCC6

Reported by: Piotr Podusowski <piotr.podusowski@…> Owned by: Douglas Gregor
Milestone: To Be Determined Component: function
Version: Boost 1.66.0 Severity: Problem
Keywords: Cc:

Description

Hello, I've been tracking a crach after upgrading GCC to 6.4 which was caused by corruption of the object stored inside boost::function.

I think that the root cause for this is in the function_buffer storage. You see, there is a char member there with the intention of "relax aliasing constraints" as it states in the comment but if my understanding of the standard is correct, it doesn't really do that. Quote from the C++:

If a program attempts to access the stored value of an object through a glvalue of other than one of the
following types the behavior is undefined: 52
— the dynamic type of the object,
— a cv-qualified version of the dynamic type of the object,
— a type similar (as defined in 4.4) to the dynamic type of the object,
— a type that is the signed or unsigned type corresponding to the dynamic type of the object,
— a type that is the signed or unsigned type corresponding to a cv-qualified version of the dynamic type
of the object,
— an aggregate or union type that includes one of the aforementioned types among its elements or non-
static data members (including, recursively, an element or non-static data member of a subaggregate
or contained union),
— a type that is a (possibly cv-qualified) base class type of the dynamic type of the object,
— a char or unsigned char type.

There is indeed a char type along other things that may be aliased safely, but function_buffer is an union with char member so it falls into:

an aggregate or union type that includes one of the **aforementioned** types

so the function_buffer itself can't be aliased. There was similar bug on GCC: 77686 (I'm unable to put links here) which was fixed by applying may_alias attribute on their function_buffer counterpart. Seems reasonable but after applying it to function_buffer it was still failing but after reading GCC docs I think I've found out why:

may_alias
Accesses through **pointers to types** with this attribute are not subject to type-
based alias analysis, but are instead assumed to be able to alias any other type
of objects. In the context of section 6.5 paragraph 7 of the C99 standard, an
lvalue expression dereferencing such a pointer is treated like having a character
type.

and there is BOOST_FUNCTION_FUNCTION::move_assign function which does:

if (this->has_trivial_copy_and_destroy())
    this->functor = f.functor;

and as it doesn't operate on the pointers, may_alias seems to be simply ignored. So two things seems to fixing it (I hope they fix it, since the bug if quite "delicate" and it's easy to hide it by changing the code that doesn't seem relevant):

1) use gnu::may_alias on function_buffer and assign it trough some helper like this instead of unions assign operator:

template<class T> void alias_safe_assign(T & dst, T & src)
{
    dst = src;
}

there is Richard's comment on GCC bugtracker: 77686#c12 where he's noted about std::swap having a references so it seems to matter.

2) operate on data member directly since it has relaxed aliasing requirements by being a char. This doesn't even require GNU extensions.:

std::memcpy(this->functor.data, f.functor.data, sizeof(boost::detail::function::function_buffer));

Of course, putting a may_alias on the functor type itself (the one that I'm assigning to boost::function object) or putting this type into function_buffer also fixes this.


I'm compiling the attached testcase using

arm-cortexa15-linux-gnueabihf-g++ (GCC) 6.4.1 20170811

and -O2 -fPIC

Br, Piotr.

Attachments (3)

bug.cpp (376 bytes ) - added by Piotr Podusowski <piotr.podusowski@…> 5 years ago.
crash.cpp (715 bytes ) - added by Joakim Tosteberg <joakim.tosteberg@…> 5 years ago.
boost_strict_aliasing_fix.patch (1013 bytes ) - added by Joakim Tosteberg <joakim.tosteberg@…> 5 years ago.
Patch to fix the issue based on suggestion in original description

Download all attachments as: .zip

Change History (8)

by Piotr Podusowski <piotr.podusowski@…>, 5 years ago

Attachment: bug.cpp added

comment:1 by Joakim Tosteberg <joakim.tosteberg@…>, 5 years ago

We have a similar issue using a gcc 6.3 compiler from https://github.com/Broadcom/stbgcc-6.3/releases

In this case the issue does not reproduce with the attached bug.cpp but we have an another example, attached as crash.cpp which reproduces it with this compiler version when built with -O2 (and given that the issue seems quite sensitive to even small changes in the code I'm not surprised that slightly different compiler version requires different code to reproduce it). Adding -fno-strict-aliasing when building makes the issue go away.

Also, using the suggested change to do a memcpy on the data member instead of directly assigning the functor inside function_template.hpp does seem to resolve the issue.

by Joakim Tosteberg <joakim.tosteberg@…>, 5 years ago

Attachment: crash.cpp added

by Joakim Tosteberg <joakim.tosteberg@…>, 5 years ago

Patch to fix the issue based on suggestion in original description

comment:2 by Piotr Podusowski <piotr.podusowski@…>, 5 years ago

Hi, this fix was delivered to boost develop branch: https://github.com/boostorg/function/pull/15

Br, Piotr.

comment:3 by Joakim Tosteberg <joakim.tosteberg@…>, 5 years ago

Ah, great!

Down at row 907 in the same file in assign_to_own there is a very similar assignment this->functor = f.functor; don't that one have to be updated as well?

comment:4 by Piotr Podusowski <piotr.podusowski@…>, 5 years ago

I didn't noticed it before, yes, I believe we should do that. Do you mind creating a pull request?

comment:5 by Joakim Tosteberg <joakim.tosteberg@…>, 5 years ago

Note: See TracTickets for help on using tickets.