Closed flippmoke closed 8 years ago
The error seems to be occurring in the geometry decoder portion of the geometry-visual-test.cpp
tests, for some reason even when we are not scaling by a value other then 1.0
we seem to have issues with strange floating point values. It is currently captured in the expected results of one of the geometry tests, but if you repeatedly run the tests you will find the error occurring in almost any geometry.
../test/geometry_visual_test.cpp:155: FAILED:
CHECK( expected_string == geojson_string )
with expansion:
"{"type":"MultiPolygon","coordinates":[[[[2731.501953125,2544],[2337,2731],
[2160,2520],[2378,2416],[2294,2159],[2465,2108],[2731,2544]]],[[[2294,2159],
[1944,2264],[2160,2520],[1860,2663],[1365,2311],[2032,1365],[2294,2159]]]]
,"properties":{"hash":12068844287086347182, "is_valid":true, "is_simple":true
, "message":"Geometry is valid"}}"
==
"{"type":"MultiPolygon","coordinates":[[[[2731,2544],[2337,2731],[2160,2520],
[2378,2416],[2294,2159],[2465,2108],[2731,2544]]],[[[2294,2159],[1944,2264],
[2160,2520],[1860,2663],[1365,2311],[2032,1365],[2294,2159]]]],"properties":
{"hash":12068844287086347182, "is_valid":true, "is_simple":true,
"message":"Geometry is valid"}}"
Note the 2731.501953125
value! It should just be an integer! Very strange.
I wanted to make sure it wasn't the encoder, so I added a hashing of the buffer prior to it going into encoder. The hashes are the same between the two results so it does not seem to be the encoder.
The values of the other points in the decoder also do not seem to be modified as well so it seems strange the positions the decoder would calculate would be the issue.
Additionally I have seem strange failures here and even a situation where the geojson string has had strange characters. So I am totally baffled by all of this!
/ cc @artemp
It continues to get more and more strange, the following folder was created in the test data directory:
Untracked files:
(use "git add <file>..." to include in what will be committed)
"output/counter-clockwise-polygo\003.counter-clockwise-hole/"
This is totally strange as it is from this line here:
https://github.com/mapbox/mapnik-vector-tile/blob/v2_spec_upgrade/test/geometry_visual_test.cpp#L119
Does this mean we have an issue with something in mapnik core? /cc @artemp
This all seems to be pointing to an object invalidly writing to memory, maybe I am not managing pointers correctly somewhere in the new code...
I think this is a problem in core -- using -fsanitize=address
I get:
0x606000018480 is located 0 bytes to the right of 64-byte region [0x606000018440,0x606000018480)
allocated by thread T0 here:
#0 0x1189eb40b in wrap__Znwm (/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/7.0.2/lib/darwin/libclang_rt.asan_osx_dynamic.dylib+0x4440b)
#1 0x10f495e7d in void std::__1::vector<mapnik::geometry::point<double>, std::__1::allocator<mapnik::geometry::point<double> > >::__push_back_slow_path<mapnik::geometry::point<double> const&>(mapnik::geometry::point<double> const&&&) (/Users/mthompson/projects/mapnik/mapnik-vector-tile/./build/Debug/tests+0x100240e7d)
#2 0x10f8dd906 in void mapnik::json::push_position_impl::operator()<std::__1::vector<mapnik::geometry::point<double>, std::__1::allocator<mapnik::geometry::point<double> > >, boost::optional<mapnik::geometry::point<double> > >(std::__1::vector<mapnik::geometry::point<double>, std::__1::allocator<mapnik::geometry::point<double> > >&, boost::optional<mapnik::geometry::point<double> > const&) const (/Users/mthompson/projects/mapnik/mapnik-vector-tile/./build/Debug/tests+0x100688906)
I am thinking there is an issue here:
Here is the full dump from sanitizer for the first bug. It also includes some information on my mapnik config.
I noticed that if I built mapnik with ./configure DEBUG=true
no errors would not occur. So the mystery deepens and I am not quite sure why... It should also be noted that when mapnik is built with DEBUG the memory errors also disappear...
When building mapnik not in DEBUG mode this change seems to resolve the problem...
@@ -66,7 +70,15 @@ struct push_position_impl
template <typename T0,typename T1>
result_type operator() (T0 & coords, T1 const& pos) const
{
- if (pos) coords.push_back(*pos);
+ if (pos)
+ {
+ auto const& p = *pos;
+ typename T0::value_type p1(p);
+ coords.emplace_back(p1);
+ }
}
};
however, I am very suspicious of everything currently... I am not sure why this is occurring..
Merging into v2_spec if we would like to include it in v2 release.
/cc @springmeyer @jakepruitt