lab-cosmo / librascal

A scalable and versatile library to generate representations for atomic-scale learning
https://lab-cosmo.github.io/librascal/
GNU Lesser General Public License v2.1
80 stars 17 forks source link

gcc detects some undefinied behaviour in adaptor neighborlist #257

Open felixmusil opened 4 years ago

felixmusil commented 4 years ago

Compiling with gcc-9 with -fsanitize=undefined compiler flag (see branch bugfix/undefined leads to several reference binding to null pointer. I fixed the ones related to spherical_expansion in the feat/sparse_kernel branch so only the ones related to AdaptorNeighbourList need to be fixed in the bugfix/undefined branch.

What I gather of the traceback of clion is that in line 1073 of adaptor_neigbour_list.hh (namely for (auto center : this->get_manager()) {) we try to access some cluster index that don't exist.

The other problem happens in line 485 of adaptor_neigbour_list.hh (construction with parameters in a json object) and it relates to an invalid pointer runtime error: member call on address 0x55687dfb34a0 which does not point to an object of type 'AdaptorNeighbourList'. There is a chance that this is a false positive though.

mastricker commented 4 years ago

Interesting. I am having a look.

mastricker commented 4 years ago

I can not replicate it with the existing test suite in branch bugfix/undefined using gcc (Debian 9.2.1-25) 9.2.1 20200123 No compilation error and not runtime error.

Do you have a specific example where this occurred? Did in occur in the test suite?

felixmusil commented 4 years ago

Right, I got these runtime errors compiling/running test_calculator. Also I use the GCC 9 available to Ubuntu 18.04. I did not try other compilers as I was investigating a weird behaviour happening in feat/sparse_kernel only with GCC 9

mastricker commented 4 years ago

Ok, so for now I would leave this issue, since it apparently persists, but assuming that my stated version of the compiler is newer, it is probably due to a compiler issue for the version in Ubuntu?

Which Ubuntu for that matter? 18.04 LTS?

Luthaf commented 4 years ago

Since this is exploiting undefined behavior, the exact set of optimization flags, and exact GCC version matter.

@felixmusil, did you run this in debug or relase mode? And adding -fsanitize=undefined to CMAKE_CXX_FLAGS?

mastricker commented 4 years ago

As for my case, I just branched out from bugfix/undefined from master on debian testing with gcc (Debian 9.2.1-25) 9.2.1 20200123 with the flags left as they are in that branch:

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Weffc++ -Wno-non-virtual-dtor -fsanitize=undefined")

It is defined before any other flags for compilation modes are set.

felixmusil commented 4 years ago

In release mode only, debug mode did not have the same bug so I did not test this either. I am not sure about the version of the GCC 9 and I am away from the office. Its the same version in the CI though.