Giter Club home page Giter Club logo

Comments (8)

jzmaddock avatar jzmaddock commented on August 11, 2024

Need to add last-mod-time.cpp to this list as well (msvc-12.0).

from graph.

deinst avatar deinst commented on August 11, 2024

astar_maze works for me (osx)
last-mod-time has problems with the stat call on osx (non fatal, but incorrect results). This would be easiest to fix by using the c++17 filesystem header, is that permissible?

from graph.

jzmaddock avatar jzmaddock commented on August 11, 2024

astar_maze works for me (osx)

Works for me on Ubuntu - it's just msvc where it's crashing.

Here's what I've found:

  • MSVC has an "uninitialised variable guard" in debug mode and this is causing the crash, because an uninitialised value is being passed to new[].
  • The problem is the value of map::m_barrier_grid.m_g.m_num_vertices.
  • Prior to the call of m.solve() in main() everything is fine, but during the function call initialization, msvc debug code overwrites the stack space with it's "special" value as a guard against subsequent uninitialized reads, this leads to the value of this->m_barrier_grid.m_g.m_num_vertices to change to the "uninitialized" special value and the subsequent crash.
  • Nothing else inside *this seems to be touched.
  • My assumption is that something during initialization of the maze was created on the stack so that this->m_barrier_grid.m_g.m_num_vertices goes out of scope. But I don't see how.

So at this point I'm stumped.

from graph.

jzmaddock avatar jzmaddock commented on August 11, 2024

last-mod-time has problems with the stat call on osx (non fatal, but incorrect results). This would be easiest to fix by using the c++17 filesystem header, is that permissible?

Ooops looks like I added that one to the wrong bug report: the example uses several unix-isms which will never compile on windows. So yes, either or <boost/filesystem.hpp> would be good choices IMO.

from graph.

deinst avatar deinst commented on August 11, 2024

astar_maze works for me (osx)

Even valgrind is not complaining, so I cannot test anything. I think llvm has similar tools to msvc, and I'll try to figure out what they are.

In the mean time, one thing that smells funny to me is the constructors of the member of maze. The initialization of m_barrier_grid calls create_barrier_grid which uses m_barriers which is initialized after barrier_grid, and so is uinititialized during the call to create_barrier_grid.

If this is the problem, then swapping lines 123 and 125 (the definitions of m_barrier_grid and m_barrier) so that m_barrier is constructed first should fix things.

from graph.

jzmaddock avatar jzmaddock commented on August 11, 2024

OK, there is a bug somewhere, but it's masked by some very clever named-return-value optimisations with gcc and clang. Here's what I have so far:

  • During:
maze random_maze(std::size_t x, std::size_t y) {
  maze m(x, y);

The member variable m.m_barrier_grid.m_g is of type const boost::grid_graph<2,unsigned int,unsigned int>& and "points to" the first member of m which is m.m_grid.

  • When the function returns, then the reference m.m_barrier_grid.m_g is left still pointing to the variable that was created inside the subroutine, and not new the copy that was returned.
  • When NVRO is in effect, then the address of the variable inside random_maze is the same as the one inside main and the issue is masked. GCC and Clang it appears do this all the time (and very cleverly so), while msvc only does it as an optimisation in release mode.
  • This all tracks back to filtered_graph_base which stores actually stores the reference - IMO this type should be non-copyable to prevent this type of bug (it's already non-assignable of course). I might just change it locally and see what if anything breaks.
  • I wonder if there are other issues like this lurking?

from graph.

jzmaddock avatar jzmaddock commented on August 11, 2024

Making filtered_graph_base non-copyable breaks a lot of code - there are a lot of factory functions that create filtered_graph's. filter_graph is actually documented to store a reference and mentions that care should be taken with it's use, but that still didn't stop the authors from falling into this trap.

In the mean time I have an easy fix for astar_maze.cpp which I'll push shortly...

from graph.

jzmaddock avatar jzmaddock commented on August 11, 2024

Pushed fixes for astar_maze.cpp: 32bc0e1

from graph.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.