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

Crash after bulk change of attribute value in shapefile #19699

Closed
qgib opened this issue Oct 16, 2014 · 19 comments
Closed

Crash after bulk change of attribute value in shapefile #19699

qgib opened this issue Oct 16, 2014 · 19 comments
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Crash/Data Corruption Data Provider Related to specific vector, raster or mesh data providers High Priority

Comments

@qgib
Copy link
Contributor

qgib commented Oct 16, 2014

Author Name: Dieter De Paepe (@DieterDePaepe)
Original Redmine Issue: 11422
Affected QGIS version: master
Redmine category:data_provider/ogr
Assignee: Sandro Santilli


Following the following steps will crash QGIS:

  1. Open a new QGIS project and open the attached shapefile
  2. Open the attribute table
  3. Enable editing in the attribute table
  4. Use the textbox for bulk changes to change the SUBTYPE attribute to NULL. (Use the 'Update All' button.)
  5. Save the changes
  6. QGIS crashed

This probably has something to do with the shapefile itself, as resaving the file removes the issue.


@qgib
Copy link
Contributor Author

qgib commented Oct 16, 2014

Author Name: Giovanni Manghi (@gioman)


notes:

it affects also master

it corrupts the data (qgis crashes when trying to reopen the shape)

it affects also the field calculator

it does not happens on qgis 2.2


  • version was changed from 2.4.0 to master
  • category_id was configured as Vectors
  • priority_id was changed from Normal to Severe/Regression
  • fixed_version_id was configured as Version 2.6

@qgib
Copy link
Contributor Author

qgib commented Oct 16, 2014

Author Name: Salvatore Larosa (@slarosa)


ogr (1.11.0) is crashing too:

#_0  0x00007ffff1b9b025 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#_1  0x00007ffff78638e8 in SHPReadOGRObject (hSHP=hSHP@entry=0x60de70, iShape=iShape@entry=1, psShape=0x60e970, psShape@entry=0x0)
    at shape2ogr.cpp:232
#_2  0x00007ffff786518d in SHPReadOGRFeature (hSHP=0x60de70, hDBF=0x60ea70, poDefn=0x60d6d0, iShape=1, psShape=0x0, 
    pszSHPEncoding=0x60d678 "ISO-8859-1") at shape2ogr.cpp:1018
#_3  0x00007ffff77edd1a in OGRShapeLayer::GetNextFeature (this=0x60eeb0) at ogrshapelayer.cpp:751
#_4  0x0000000000402f1d in ReportOnLayer (poLayer=poLayer@entry=0x60eeb0, pszWHERE=pszWHERE@entry=0x0, 
    pszGeomField=pszGeomField@entry=0x0, poSpatialFilter=poSpatialFilter@entry=0x0) at ogrinfo.cpp:557
#_5  0x00000000004026e3 in main (nArgc=<optimized out>, papszArgv=0x60dda0) at ogrinfo.cpp:334

this should be addressed to gdal bug tracker unless it was already fixed.

I am getting the crash on 2.2, 2.4, master and 1.8

Giovanni your 2.2 which gdal version was compiled with?


  • priority_id was changed from Severe/Regression to Normal
  • category_id was changed from Vectors to Data Provider/OGR
  • status_id was changed from Open to Feedback

@qgib
Copy link
Contributor Author

qgib commented Oct 16, 2014

Author Name: Giovanni Manghi (@gioman)


Giovanni your 2.2 which gdal version was compiled with?

I tested on my Windows VM, so I tested qgis 2.2 installed with the standalone installer (and of course the gdal version it was shipped with it).


  • status_id was changed from Feedback to Open

@qgib
Copy link
Contributor Author

qgib commented Oct 30, 2014

Author Name: Giovanni Manghi (@gioman)


  • priority_id was changed from Normal to High

@qgib
Copy link
Contributor Author

qgib commented Oct 30, 2014

Author Name: Giovanni Manghi (@gioman)


I confirm the regression (it is also affected the field calculator).


  • priority_id was changed from High to Severe/Regression

@qgib
Copy link
Contributor Author

qgib commented Oct 30, 2014

Author Name: Jürgen Fischer (@jef-n)


Giovanni Manghi wrote:

I confirm the regression (it is also affected the field calculator).

ogrinfo shows it's a @3d Line string@ shape with all Z coordinate @too_big@. After the update, QGIS built with 1.11.1 crashes when reading the file, but so does ogrinfo 1.11.1. ogrinfo 1.10.1 however still lists it (although all Z coordinates are reported 0 after the update) and QGIS built against 1.10.1 also shows it. Smells like a GDAL regression, although I just tried 2.0.1 and didn't build master against 1.10.1.


  • priority_id was changed from Severe/Regression to Normal
  • resolution was changed from to up/downstream

@qgib
Copy link
Contributor Author

qgib commented Oct 30, 2014

Author Name: Giovanni Manghi (@gioman)


Jürgen Fischer wrote:

Giovanni Manghi wrote:

I confirm the regression (it is also affected the field calculator).

ogrinfo shows it's a @3d Line string@ shape with all Z coordinate @too_big@. After the update, QGIS built with 1.11.1 crashes when reading the file, but so does ogrinfo 1.11.1. ogrinfo 1.10.1 however still lists it (although all Z coordinates are reported 0 after the update) and QGIS built against 1.10.1 also shows it. Smells like a GDAL regression, although I just tried 2.0.1 and didn't build master against 1.10.1.

should we close this?

@qgib
Copy link
Contributor Author

qgib commented Oct 31, 2014

Author Name: Jürgen Fischer (@jef-n)


  • fixed_version_id was changed from Version 2.6 to Future Release - High Priority

@qgib
Copy link
Contributor Author

qgib commented May 27, 2015

Author Name: Giovanni Manghi (@gioman)


  • priority_id was changed from Normal to High

@qgib
Copy link
Contributor Author

qgib commented Jan 19, 2016

Author Name: Sandro Santilli (@strk)


Still happens as of 69cb0c4 (2.14-pre) built and running against GDAL 1.11.1

Backtrace:

#0  __memcpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:33
#_1  0x00007f0c2d8eb858 in SHPReadOGRObject (hSHP=hSHP@entry=0x7f0b6c00cc00, iShape=iShape@entry=1, psShape=psShape@entry=0x7f0b6c019080)
    at shape2ogr.cpp:232
#_2  0x00007f0c2d8ed1fd in SHPReadOGRFeature (hSHP=0x7f0b6c00cc00, hDBF=0x7f0b6c0299c0, poDefn=0x7f0b6c029cf0, iShape=1, psShape=0x7f0b6c019080, 
    pszSHPEncoding=0x7f0c30fe63d8 <std::string::_Rep::_S_empty_rep_storage+24> "") at shape2ogr.cpp:1018
#_3  0x00007f0c2d85f73a in OGRShapeLayer::GetNextFeature (this=0x7f0b6c00dea0) at ogrshapelayer.cpp:751
#_4  0x00007f0b99e1a494 in QgsOgrFeatureIterator::fetchFeature (this=0x7f0b6c009b90, feature=...)
    at /usr/src/qgis/qgis-master/src/providers/ogr/qgsogrfeatureiterator.cpp:213
#_5  0x00007f0c32585bbf in QgsAbstractFeatureIterator::nextFeature (this=0x7f0b6c009b90, f=...)
    at /usr/src/qgis/qgis-master/src/core/qgsfeatureiterator.cpp:76
#_6  0x00007f0c3248f01e in QgsFeatureIterator::nextFeature (this=0x7f0b6c00d2c8, f=...)
    at /usr/src/qgis/qgis-master/src/core/qgsfeatureiterator.h:234
#_7  0x00007f0c3270f8c5 in QgsVectorLayerFeatureIterator::fetchFeature (this=0x7f0b6c00d170, f=...)
    at /usr/src/qgis/qgis-master/src/core/qgsvectorlayerfeatureiterator.cpp:237
#_8  0x00007f0c32585bbf in QgsAbstractFeatureIterator::nextFeature (this=0x7f0b6c00d170, f=...)
    at /usr/src/qgis/qgis-master/src/core/qgsfeatureiterator.cpp:76
#_9  0x00007f0c3248f01e in QgsFeatureIterator::nextFeature (this=0x7f0b7effc8c0, f=...)
    at /usr/src/qgis/qgis-master/src/core/qgsfeatureiterator.h:234
#_10 0x00007f0c32721684 in QgsVectorLayerRenderer::drawRendererV2 (this=0x2f604b0, fit=...)
    at /usr/src/qgis/qgis-master/src/core/qgsvectorlayerrenderer.cpp:290

@qgib
Copy link
Contributor Author

qgib commented Jan 19, 2016

Author Name: Sandro Santilli (@strk)


  • status_id was changed from Open to In Progress
  • assigned_to_id was configured as Sandro Santilli

@qgib
Copy link
Contributor Author

qgib commented Jan 19, 2016

Author Name: Sandro Santilli (@strk)


Filed GDAL ticket here: https://trac.osgeo.org/gdal/ticket/6317

@qgib
Copy link
Contributor Author

qgib commented Jan 19, 2016

Author Name: Sandro Santilli (@strk)


GDAL is responsible for the crash on reading, but I still don't know who's responsible for corruption on writing

@qgib
Copy link
Contributor Author

qgib commented Jan 19, 2016

Author Name: Sandro Santilli (@strk)


So the GDAL bug is that the M values are dropped when updating XYM geometries, but the output is still advertised as containing them, which makes the reader crash.
If we want to implement a workaround on our side we need to drop the M value upfront, when we know the version of GDAL in use is bogus in that reguard.

Note that if the input is an XYZM shapefile the M is dropped anyway.

@qgib
Copy link
Contributor Author

qgib commented Jan 19, 2016

Author Name: Even Rouault (@rouault)


Will be fixed in upcoming GDAL 1.11.4 & 2.0.2

@qgib
Copy link
Contributor Author

qgib commented Jan 19, 2016

Author Name: Sandro Santilli (@strk)


I confirm current GDAL trunk fixes the issue.

A workaround is not trival, maybe we should just refuse to allow editing of XYM input shapefiles IFF GDAL version is known to be broken, and be noisy about it.
Worth it ?


  • status_id was changed from In Progress to Feedback

@qgib
Copy link
Contributor Author

qgib commented Jan 19, 2016

Author Name: Sandro Santilli (@strk)


So now that I finally got it I'll leave a note here:
OGR will not help in knowing if the input is or not affected by the bug in that it will transparently map the XYM input as an XYZ one, so to use the input will look like a normal XYZ. But will produce a corrupted output or not based on whether the actual shapefile being read was an XYM or a real XYZ, which we could only check by independently checking the shapefile input. Then, once the condition is spot, we should be rewriting the shapefile into a clean XYZ or XY in order to deal with it.

A lot of work for a workaround (would imply designing an interface to let the user know what we're doing with her data etc.).

@qgib
Copy link
Contributor Author

qgib commented Jan 19, 2016

Author Name: Sandro Santilli (@strk)


closing as fixed upstream -- remember that the M value is still lost, if you want to retain it it'd still take a refactoring in OGR.


  • status_id was changed from Feedback to Closed

@qgib
Copy link
Contributor Author

qgib commented Jan 19, 2016

Author Name: Sandro Santilli (@strk)


I filed #22144 to not loose track of the lost-M issue

@qgib qgib added Bug Either a bug report, or a bug fix. Let's hope for the latter! High Priority Data Provider Related to specific vector, raster or mesh data providers Crash/Data Corruption labels May 25, 2019
@qgib qgib added this to the Future Release - High Priority milestone May 25, 2019
@qgib qgib closed this as completed May 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Crash/Data Corruption Data Provider Related to specific vector, raster or mesh data providers High Priority
Projects
None yet
Development

No branches or pull requests

1 participant