Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unaligned memory access in order_by #355

Closed
RelayrMA opened this issue Dec 3, 2020 · 10 comments
Closed

Unaligned memory access in order_by #355

RelayrMA opened this issue Dec 3, 2020 · 10 comments

Comments

@RelayrMA
Copy link

RelayrMA commented Dec 3, 2020

We're running our unit tests with undefined behaviour sanitizer and it has caught the following misaligned access:

/home/m/.conan/data/sqlpp11/0.60/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/sqlpp11/order_by.h:246:74: runtime error: reference binding to misaligned address 0x7fffffffd02a for type 'const struct statement_t', which requires 8 byte alignment
0x7fffffffd02a: note: pointer points here
 00 00  58 6c 28 57 55 55 00 00  00 00 00 00 00 00 00 00  68 8d f0 f5 ff 7f 00 00  68 9a f0 f5 ff 7f
              ^ 
    #0 0x55555705bd08 in sqlpp::new_statement_impl<sqlpp::consistent_t, sqlpp::detail::statement_policies_t<void, sqlpp::no_with_t, sqlpp::select_t, sqlpp::no_select_flag_list_t, sqlpp::select_column_list_t<void, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Type>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::CreateTs>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::ReceiveTs>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::DeviceId>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Payload> >, sqlpp::from_t<void, azure::db::Messages>, sqlpp::where_t<void, sqlpp::binary_expression_t<sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id>, sqlpp::op::greater, sqlpp::integral_operand> >, sqlpp::no_group_by_t, sqlpp::no_having_t, sqlpp::no_order_by_t, sqlpp::no_limit_t, sqlpp::no_offset_t, sqlpp::no_union_t, sqlpp::no_for_update_t>, sqlpp::no_order_by_t, sqlpp::order_by_t<void, sqlpp::sort_order_t<sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id> > > >::type sqlpp::no_order_by_t::_base_t<sqlpp::detail::statement_policies_t<void, sqlpp::no_with_t, sqlpp::select_t, sqlpp::no_select_flag_list_t, sqlpp::select_column_list_t<void, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Type>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::CreateTs>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::ReceiveTs>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::DeviceId>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Payload> >, sqlpp::from_t<void, azure::db::Messages>, sqlpp::where_t<void, sqlpp::binary_expression_t<sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id>, sqlpp::op::greater, sqlpp::integral_operand> >, sqlpp::no_group_by_t, sqlpp::no_having_t, sqlpp::no_order_by_t, sqlpp::no_limit_t, sqlpp::no_offset_t, sqlpp::no_union_t, sqlpp::no_for_update_t> >::_order_by_impl<void, sqlpp::sort_order_t<sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id> > >(sqlpp::consistent_t, sqlpp::sort_order_t<sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id> >) const /home/m/.conan/data/sqlpp11/0.60/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/sqlpp11/order_by.h:246
    #1 0x5555570580fc in sqlpp::new_statement_impl<sqlpp::check_order_by<sqlpp::sort_order_t<sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id> > >::type, sqlpp::detail::statement_policies_t<void, sqlpp::no_with_t, sqlpp::select_t, sqlpp::no_select_flag_list_t, sqlpp::select_column_list_t<void, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Type>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::CreateTs>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::ReceiveTs>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::DeviceId>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Payload> >, sqlpp::from_t<void, azure::db::Messages>, sqlpp::where_t<void, sqlpp::binary_expression_t<sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id>, sqlpp::op::greater, sqlpp::integral_operand> >, sqlpp::no_group_by_t, sqlpp::no_having_t, sqlpp::no_order_by_t, sqlpp::no_limit_t, sqlpp::no_offset_t, sqlpp::no_union_t, sqlpp::no_for_update_t>, sqlpp::no_order_by_t, sqlpp::order_by_t<void, sqlpp::sort_order_t<sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id> > > >::type sqlpp::no_order_by_t::_base_t<sqlpp::detail::statement_policies_t<void, sqlpp::no_with_t, sqlpp::select_t, sqlpp::no_select_flag_list_t, sqlpp::select_column_list_t<void, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Type>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::CreateTs>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::ReceiveTs>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::DeviceId>, sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Payload> >, sqlpp::from_t<void, azure::db::Messages>, sqlpp::where_t<void, sqlpp::binary_expression_t<sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id>, sqlpp::op::greater, sqlpp::integral_operand> >, sqlpp::no_group_by_t, sqlpp::no_having_t, sqlpp::no_order_by_t, sqlpp::no_limit_t, sqlpp::no_offset_t, sqlpp::no_union_t, sqlpp::no_for_update_t> >::order_by<sqlpp::sort_order_t<sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id> > >(sqlpp::sort_order_t<sqlpp::column_t<azure::db::Messages, azure::db::Messages_::Id> >) const /home/m/.conan/data/sqlpp11/0.60/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/sqlpp11/order_by.h:221
(rest of backtrace is our code)

While executing

db(select(all_of(messages))
		.from(messages)
		.where(messages.id > minId)
		.order_by(messages.id.asc())
		.limit(limit)));
@rbock
Copy link
Owner

rbock commented Dec 4, 2020

That is interesting.

It is complaining about a static_cast to the derived class in a variadic CRTP. We might need a language lawyer to see if that is valid or not?

@RelayrMA
Copy link
Author

RelayrMA commented Dec 4, 2020

I haven't seen CRTP before and I have hard time following the code.
Considering https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#issue-suppression I think a language lawyer may be indeed needed...

@rbock
Copy link
Owner

rbock commented Dec 5, 2020

This is a drastically simplified version of what is happening:

#include <iostream>
#include <string_view>

template<typename Derived>
struct Base1
{
    constexpr auto& get1() const
    {
        return static_cast<const Derived&>(*this);
    }
};

template<typename Derived>
struct Base2
{
    constexpr auto& get2() const
    {
        return static_cast<const Derived&>(*this);
    }
};

template<typename Derived>
struct Base3
{
    constexpr auto& get3() const
    {
        return static_cast<const Derived&>(*this);
    }
};


struct X : public Base1<X>, public Base2<X>, public Base3<X>
{
    constexpr X(std::string_view d) : data{d} {}

    std::string_view data;
};


int main()
{
    constexpr auto x = X{"cheesecake"};
    static_assert(x.get1().data == x.get2().data);
    static_assert(&x.get1() == &x.get2());
    static_assert(&x.get1() == &x.get3());

    std::cout << "Base1: " << alignof(Base1<X>) << std::endl; // 1
    std::cout << "Base2: " << alignof(Base2<X>) << std::endl; // 1
    std::cout << "Base3: " << alignof(Base3<X>) << std::endl; // 1
    std::cout << "X    : " << alignof(X) << std::endl;        // 8
}

This is using C++17, but that's just to make the code slightly more concise.

Judging by the static_asserts, this seems to be OK despite the different alignments (I'll need to experiment with UB sanitizer, though).

@rbock
Copy link
Owner

rbock commented Dec 6, 2020

I tried to reproduce your code sample:

#include <iostream>
#include <sqlpp11/sqlpp11.h>

#include <MockDb.h>
#include <Sample.h>

int main()
{
    const auto t = test::TabBar{};
    auto db = MockDb{};
    for (const auto& row : db(select(all_of(t)).from(t).where(t.alpha > 15).order_by(t.alpha.asc()).limit(100u)))
    {
        std::cout << row.alpha;
    }
}

Compiled with clang-12

clang++ -std=c++11 -stdlib=libc++ -fsanitize=undefined -I include -I tests -I ../date/include

I got no alignment warnings when running it. I also tried with -O0.

Can you create a minimal viable example of the problem?

@RelayrMA
Copy link
Author

RelayrMA commented Dec 6, 2020

Here's a sample which produces the issue and then crashes:

g++ 1.cpp -Isqlpp11-connector-sqlite3/include -I/home/m/.conan/data/sqlpp11/0.60/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include -I/home/m/.conan/data/date/2.4.1/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include -fsanitize=undefined -o 1 sqlpp11-connector-sqlite3/src/libsqlpp11-connector-sqlite3.a -lsqlite3 && ./1
/home/m/.conan/data/sqlpp11/0.60/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/sqlpp11/order_by.h:246:74: runtime error: reference binding to misaligned address 0x7ffedd5110ca for type 'const struct statement_t', which requires 8 byte alignment
0x7ffedd5110ca: note: pointer points here
 00 00  f0 9e b3 e5 3b 56 00 00  32 13 51 dd fe 7f 00 00  2e 13 51 dd fe 7f 00 00  00 00 00 00 00 00
              ^ 
/home/m/.conan/data/sqlpp11/0.60/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/include/sqlpp11/order_by.h:246:74: runtime error: load of misaligned address 0x7ffedd5110ca for type 'const struct statement_t', which requires 8 byte alignment
0x7ffedd5110ca: note: pointer points here
 00 00  f0 9e b3 e5 3b 56 00 00  32 13 51 dd fe 7f 00 00  2e 13 51 dd fe 7f 00 00  00 00 00 00 00 00
              ^ 
terminate called after throwing an instance of 'sqlpp::exception'
  what():  Sqlite3 error: Could not prepare statement: no such table: messages (statement was >>SELECT messages.id,messages.type,messages.create_ts,messages.receive_ts,messages.device_id,messages.payload FROM messages WHERE (messages.id>0) ORDER BY messages.id ASC LIMIT 1<<

[1]    29301 abort      ./1

The issue occurs when I compile with gcc 9.3.0 but not with clang 11.0.
1.zip

@RelayrMA
Copy link
Author

RelayrMA commented Dec 6, 2020

BTW, one could add one more assertion that also passes:
static_assert(&x.get1() == &x);

@rbock
Copy link
Owner

rbock commented Dec 6, 2020

Thanks for the example! I can reproduce it with gcc-11, too.

I'll try to create a minimal example that exposes the behavior and then start looking for a language lawyer.

@rbock
Copy link
Owner

rbock commented Dec 7, 2020

I found a minimal example and posted the question on stackoverflow.

@rbock
Copy link
Owner

rbock commented Dec 17, 2020

The answer on stackoverflow seems very plausible to me. I filed a bug for gcc.

@rbock rbock closed this as completed Dec 17, 2020
@rbock
Copy link
Owner

rbock commented Jan 1, 2021

FYI: The gcc sanitizer bug is now fixed in trunk: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98206

I tested with the example above: Works fine now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants