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

Variant_test fails with boost 1.73.0 #3711

Closed
hirschsn opened this issue May 12, 2020 · 2 comments · Fixed by #3725
Closed

Variant_test fails with boost 1.73.0 #3711

hirschsn opened this issue May 12, 2020 · 2 comments · Fixed by #3725
Assignees
Labels

Comments

@hirschsn
Copy link
Contributor

hirschsn commented May 12, 2020

The unit test Variant_test fails with the most recent version of boost 1.73.0 in the unpack_pair test. The const std::vector<Variant> &v passed to unpack_pair (Variant.hpp:79) contains garbage. :)

The reason is some fancy template gone wrong in the script interface. Compiler warning:

[...]
/home/hirschsn/esp/espresso-add/src/script_interface/constraints/initialize.cpp:67:79:   required from here
/home/hirschsn/esp/espresso-add/src/script_interface/Variant.hpp:75:29: warning: narrowing conversion of ‘(double)pair.std::pair<int, double>::second’ from ‘double’ to ‘boost::enable_if_c<true, bool>::type’ {aka ‘bool’} [-Wnarrowing]
   75 |   return {{pair.first, pair.second}};
      |                        ~~~~~^~~~~~

Latest ESPResSo python branch.
Edit: Forgot to mention: Of course, with boost 1.72.0 everything works fine.

@jngrad
Copy link
Member

jngrad commented May 19, 2020

Thanks for bringing our attention to this issue. Boost 1.73 will be shipped with Fedora 33, which will start rebuilding all packages including espressomd on July 22. I can maybe have a look next week.

@jngrad
Copy link
Member

jngrad commented May 22, 2020

Just tried it with boost 1.73 on Ubuntu 20.04 with espresso 4.2-dev and 4.1.2. The Variant test fails because the pair is serialized into a vector of size 1. I originally also got random values as I forgot to add boost::get<T>() when printing the first value of the vector. The second value is uninitialized memory, so it can only be random.

The double brace expression return {{pair.first, pair.second}}; instantiates a vector of 1 Variant with underlying type int instead of a vector of 2 Variants with underlying types int and double, therefore unpack_pair() accesses memory out of bounds when fetching the second element. That's an easy fix, PR is coming.

@jngrad jngrad added this to the Espresso 4.1.3 milestone May 22, 2020
@jngrad jngrad self-assigned this May 22, 2020
@jngrad jngrad added the Bug label May 22, 2020
@kodiakhq kodiakhq bot closed this as completed in #3725 May 22, 2020
kodiakhq bot added a commit that referenced this issue May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants