Bug report #12416

QGIS 2.8.1 crash opening FileGDB (openGDB-Driver)

Added by Flo Ju about 2 years ago. Updated about 1 year ago.

Status:Closed Start Date:03/20/2015
Priority:High Due date:
Assigned to:Sandro Santilli % Done:

100%

Category:OGR Data Provider
Target version:Version 2.14
Platform:Windows , Linux Pull Request or Patch supplied:Yes
Platform version:Win 7 x64, Ubuntu 14.04 64bit Affected version:master
Status info: Causes crash or corruption:Yes
Resolution:fixed/implemented Tag:

Description

Opening a FileGDB in QGIS 2.8.1. (Win 7 x64) causes a crash of the application. With QGIS 2.4. no problems.
Crash-Dump attached.

qgis-20150320-095230-3012-3620-exported.zip - Dump-File Crash (3.6 MB) Flo Ju, 03/20/2015 02:00 am

Trecks.gdb.zip - FileGeodatabase zipped (1.6 MB) Flo Ju, 03/22/2015 11:30 pm

valgrind.txt (439 kB) Sandro Santilli, 01/19/2016 09:21 am

Associated revisions

Revision bc4d16e8d912d9f6a605daa02d2959cd8145e770
Added by Sandro Santilli over 1 year ago

Check WKB boundaries while simplifying for rendering

Fixes crash on simplifying mixed-dimension collections.
Closes #12416

Revision 87887e43d191caef5f5c83f6f2bd2d829328ab0d
Added by Sandro Santilli about 1 year ago

Add test for unexpected WKB input in simplification

Closes #12416

History

Updated by Giovanni Manghi about 2 years ago

  • Status changed from Open to Feedback
  • Causes crash or corruption changed from No to Yes

I cannot confirm (qgis 2.8.1 on Win 7 64bit). I tested using the file geodatabases downloaded from here.

http://gapanalysis.usgs.gov/padus/data/download/

Please attach a sample of the data that causes the crash.

Updated by Flo Ju about 2 years ago

As an attachement the GDB-File (zipped) - QGIS 2.8.1 crashes on Win-Server VM and Win7 64Bit when opening the layer Trecks_3D

Updated by Giovanni Manghi about 2 years ago

Flo Ju wrote:

As an attachement the GDB-File (zipped) - QGIS 2.8.1 crashes on Win-Server VM and Win7 64Bit when opening the layer Trecks_3D

it opens just fine here.

Updated by Flo Ju about 2 years ago

We tried it on different systems: Win7 64Bit SP1 (3 different systems) and Win 2008 Server VM --> always the same crash. If I open the same GDB in QGIS 2.8.1. (with openGDB-Driver) on a Linux-system it works fine.

Updated by Giovanni Manghi about 2 years ago

Flo Ju wrote:

We tried it on different systems: Win7 64Bit SP1 (3 different systems) and Win 2008 Server VM --> always the same crash. If I open the same GDB in QGIS 2.8.1. (with openGDB-Driver) on a Linux-system it works fine.

could you try see if ogrinfo works against this problematic layer? use the osgeo4w shell, to be sure you are using the same gdal/ogr installation qgis uses. Thanks.

Updated by Flo Ju about 2 years ago

Hi! OGR on command-line is 11.1.2 - the version used in QGIS 2.8.1 is the same (QGIS-Info). Tried to convert a Feature Layer from the GDB with ogr2ogr to a SHP and it worked fine.

Updated by Giovanni Manghi about 2 years ago

Flo Ju wrote:

Hi! OGR on command-line is 11.1.2 - the version used in QGIS 2.8.1 is the same (QGIS-Info). Tried to convert a Feature Layer from the GDB with ogr2ogr to a SHP and it worked fine.

I'm out of ideas. The system you tested do share something that can differ from a "common" installation? cheers.

Updated by Jürgen Fischer over 1 year ago

  • Category changed from Vectors to OGR Data Provider

Updated by Giovanni Manghi over 1 year ago

I have also tested some very large filegdb dataset linked here #11746.6 with no issues whatsoever.

I suggest to close this ticket as probable local issue.

Updated by clifford snow over 1 year ago

Giovanni Manghi wrote:

I have also tested some very large filegdb dataset linked here

http://hub.qgis.org/issues/11746#note-6

with no issues whatsoever.

I suggest to close this ticket as probable local issue.

I was just looking for an open ticket on this subject. QGIS closes unexpectedly when attempting to open a 124Mb Filegdb file [1]. QGIS seems to finish rendering the ways just before crashing. I'm running 1.12-1 lyon on a macbook pro with 16gb of memory.

org2ogr is able to convert to a shapefile which QGIS loads successfully.

[1] https://fortress.wa.gov/dnr/adminsa/gisdata/datadownload/state_roads.zip

Updated by Giovanni Manghi over 1 year ago

I was just looking for an open ticket on this subject. QGIS closes unexpectedly when attempting to open a 124Mb Filegdb file [1]. QGIS seems to finish rendering the ways just before crashing. I'm running 1.12-1 lyon on a macbook pro with 16gb of memory.

org2ogr is able to convert to a shapefile which QGIS loads successfully.

[1] https://fortress.wa.gov/dnr/adminsa/gisdata/datadownload/state_roads.zip

and again no problems whatsoever in my Windows 7 testing machine with QGIS master, ltr and 2.12...

Can you please guys try a fresh installation? uninstall, remove .qgis, clean config, re-install.

Updated by Giovanni Manghi over 1 year ago

  • Status changed from Feedback to Open

Flo Ju wrote:

As an attachement the GDB-File (zipped) - QGIS 2.8.1 crashes on Win-Server VM and Win7 64Bit when opening the layer Trecks_3D

you are right, if using the open driver qgis crashes (but gdal/ogr is ok).

I also replicated with other datasets, but not all.

And if using the closed driver I never got a crash with any dataset.

Updated by Giovanni Manghi over 1 year ago

  • Affected version changed from 2.8.1 to master

Updated by Sandro Santilli over 1 year ago

  • Platform changed from Windows to Windows , Linux
  • Platform version changed from Win 7 x64 to Win 7 x64, Ubuntu 14.04 64bit

I can reproduce under Ubuntu 14.04 64bit with GDAL trunk (2.1.0dev) and OpenFileGDB.
`ogrinfo -al` works fine.

Updated by Sandro Santilli over 1 year ago

#0  __memcpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:158
#1  0x00007f1d0386184e in QgsMapToPixelSimplifier::simplifyWkbGeometry (simplifyFlags=3, wkbType=QGis::WKBLineString, 
    sourceWkb=0x7f1c5023809b "\325V\354/|\237@\340u\236\232\001\255\026A\300\233\234\352\333H\tA", sourceWkbSize=68191865, 
    targetWkb=0x7f1c501d54ab "\265\246yGʢ@", targetWkbSize=@0x7f1c56de0350: 9, envelope=..., map2pixelTol=1224.0509582740297, writeHeader=true, 
    isaLinearRing=false) at /usr/src/qgis/qgis-master/src/core/qgsmaptopixelgeometrysimplifier.cpp:276
#2  0x00007f1d03861f42 in QgsMapToPixelSimplifier::simplifyWkbGeometry (simplifyFlags=3, wkbType=QGis::WKBMultiLineString, 
    sourceWkb=0x7f1c501c6112 "@\252\366?Pg\bA", sourceWkbSize=25371, targetWkb=0x7f1c501d54a2 "@", targetWkbSize=@0x7f1c56de0448: 10370, 
    envelope=..., map2pixelTol=34.986439634150109, writeHeader=true, isaLinearRing=false)
    at /usr/src/qgis/qgis-master/src/core/qgsmaptopixelgeometrysimplifier.cpp:408
#3  0x00007f1d03862209 in QgsMapToPixelSimplifier::simplifyGeometry (geometry=0x7f1c50186300, simplifyFlags=3, tolerance=34.986439634150109)
    at /usr/src/qgis/qgis-master/src/core/qgsmaptopixelgeometrysimplifier.cpp:460
#4  0x00007f1d038622ea in QgsMapToPixelSimplifier::simplifyGeometry (this=0x7f1c50196ec0, geometry=0x7f1c50186300)
    at /usr/src/qgis/qgis-master/src/core/qgsmaptopixelgeometrysimplifier.cpp:475
#5  0x00007f1d037e5507 in QgsAbstractFeatureIterator::simplify (this=0x7f1c5000ede0, feature=...)
    at /usr/src/qgis/qgis-master/src/core/qgsfeatureiterator.cpp:216
#6  0x00007f1d037e4c1c in QgsAbstractFeatureIterator::nextFeature (this=0x7f1c5000ede0, f=...)
    at /usr/src/qgis/qgis-master/src/core/qgsfeatureiterator.cpp:85

Updated by Sandro Santilli over 1 year ago

  • Status changed from Open to In Progress
  • Assigned to set to Sandro Santilli

Updated by Sandro Santilli over 1 year ago

There are a couple of these errors before the crash:


==10779== Thread 11 Thread (pooled):
==10779== Invalid read of size 8
==10779==    at 0x659F71E: QgsMapToPixelSimplifier::simplifyWkbGeometry(int, QGis::WkbType, unsigned char const*, int, unsigned char*, int&, QgsRectangle const&, double, bool, bool) (qgsmaptopixelgeometrysimplifier.cpp:264)
==10779==    by 0x659FF41: QgsMapToPixelSimplifier::simplifyWkbGeometry(int, QGis::WkbType, unsigned char const*, int, unsigned char*, int&, QgsRectangle const&, double, bool, bool) (qgsmaptopixelgeometrysimplifier.cpp:408)
==10779==    by 0x65A0208: QgsMapToPixelSimplifier::simplifyGeometry(QgsGeometry*, int, double) (qgsmaptopixelgeometrysimplifier.cpp:460)
==10779==    by 0x65A02E9: QgsMapToPixelSimplifier::simplifyGeometry(QgsGeometry*) const (qgsmaptopixelgeometrysimplifier.cpp:475)
==10779==    by 0x6523506: QgsAbstractFeatureIterator::simplify(QgsFeature&) (qgsfeatureiterator.cpp:216)
==10779==    by 0x6522C1B: QgsAbstractFeatureIterator::nextFeature(QgsFeature&) (qgsfeatureiterator.cpp:85)
==10779==    by 0x642C02D: QgsFeatureIterator::nextFeature(QgsFeature&) (qgsfeatureiterator.h:234)
==10779==    by 0x66AC8D4: QgsVectorLayerFeatureIterator::fetchFeature(QgsFeature&) (qgsvectorlayerfeatureiterator.cpp:237)
==10779==    by 0x6522BCE: QgsAbstractFeatureIterator::nextFeature(QgsFeature&) (qgsfeatureiterator.cpp:76)
==10779==    by 0x642C02D: QgsFeatureIterator::nextFeature(QgsFeature&) (qgsfeatureiterator.h:234)
==10779==    by 0x66BE693: QgsVectorLayerRenderer::drawRendererV2(QgsFeatureIterator&) (qgsvectorlayerrenderer.cpp:290)
==10779==    by 0x66BDBE3: QgsVectorLayerRenderer::render() (qgsvectorlayerrenderer.cpp:252)
==10779==  Address 0x93f63e3b is 0 bytes after a block of size 25,371 alloc'd
==10779==    at 0x4C2B7AA: operator new[](unsigned long) (vg_replace_malloc.c:392)
==10779==    by 0x83A75015: QgsOgrFeatureIterator::readFeature(void*, QgsFeature&) (qgsogrfeatureiterator.cpp:340)
==10779==    by 0x83A7474D: QgsOgrFeatureIterator::fetchFeature(QgsFeature&) (qgsogrfeatureiterator.cpp:215)
==10779==    by 0x6522BCE: QgsAbstractFeatureIterator::nextFeature(QgsFeature&) (qgsfeatureiterator.cpp:76)
==10779==    by 0x642C02D: QgsFeatureIterator::nextFeature(QgsFeature&) (qgsfeatureiterator.h:234)
==10779==    by 0x66AC8D4: QgsVectorLayerFeatureIterator::fetchFeature(QgsFeature&) (qgsvectorlayerfeatureiterator.cpp:237)
==10779==    by 0x6522BCE: QgsAbstractFeatureIterator::nextFeature(QgsFeature&) (qgsfeatureiterator.cpp:76)
==10779==    by 0x642C02D: QgsFeatureIterator::nextFeature(QgsFeature&) (qgsfeatureiterator.h:234)
==10779==    by 0x66BE693: QgsVectorLayerRenderer::drawRendererV2(QgsFeatureIterator&) (qgsvectorlayerrenderer.cpp:290)
==10779==    by 0x66BDBE3: QgsVectorLayerRenderer::render() (qgsvectorlayerrenderer.cpp:252)
==10779==    by 0x659696B: QgsMapRendererParallelJob::renderLayerStatic(LayerRenderJob&) (qgsmaprendererparalleljob.cpp:234)
==10779==    by 0x6597CDF: QtConcurrent::FunctionWrapper1<void, LayerRenderJob&>::operator()(LayerRenderJob&) (qtconcurrentfunctionwrappers.h:86)
==10779== 

Full valgrind log is atached

Updated by Sandro Santilli over 1 year ago

The crash only happens when the offending layer is the first one on the map. Crash also occurs by loading the "Tracks" layer.

Updated by Even Rouault over 1 year ago

The root cause is a bug in the OpenFileGDB driver (https://trac.osgeo.org/gdal/ticket/6332) that results in inconsistant WKB export, that QGIS crashes on.

Updated by Sandro Santilli over 1 year ago

Thanks Even.
Still I think QGIS should check before reading past the end of the buffer.
I'll look into that in the evening.

Updated by Sandro Santilli over 1 year ago

My take on boundary checking is here: https://github.com/qgis/QGIS/pull/2721
At the time of writing this comment, the code checks just enough to not fail in this case.
It's to be noted that the main QGIS WKB parser has no problem with the "malformed" WKB, only the WKB parser used inside the QgsMapToPixelSimplifier class, which for some reason acts on the WKB format (maybe it should just really stop doing that).

Updated by Sandro Santilli over 1 year ago

  • Pull Request or Patch supplied changed from No to Yes

Updated by Sandro Santilli over 1 year ago

I'm taking back the "main QGIS WKB parser has no problem with the malformed WKB" as that's very unlikely as the QgsGeometry::fromWkb() method does not even use the passed WKB size argument to check against reading past the input end (is implemented by QgsAbstractGeometryV2::fromWkb not even taking that size parameter)

Updated by Sandro Santilli over 1 year ago

The other possibility is that QGIS centralized WKB parser simply handles better the parsing of geometry collections, NOT ASSUMING the dimensionality advertised for the compound object is the same of the one for the elements.

Updated by Sandro Santilli over 1 year ago

I've handled to make QgsGeometry.fromWkb mess up with memory using this input:

010700000002000000010200008002000000000000000000000000000000000000000000000000000000000000000000000000000000000000008DEDB5A0F7C6B0BE010200008002000000000000000000000000000000000000000000000000000000000000000000000000000000000000008DEDB5A0F7C6B03E

Now working on a proper testcase to secure the later fix

Updated by Sandro Santilli over 1 year ago

So the code showing unrobust WKB parser is here: https://github.com/qgis/QGIS/pull/2722
It confirms the internal WKB parser also needs attention.

Updated by Sandro Santilli over 1 year ago

Beside boundary checking, the problem is with the parser inside geometry-simplifier just skipping 5 bytes in each component WKB, rather than reading them. They do contain the actual sub-geometry type, which in this case does not have the same dimensionality of the container. I think this bug can be closed as soon as the boundary checking is in place (fixing the crash). Further improvements should probably involve using the WkbPtr (once it also has boundary checking).

Updated by Sandro Santilli over 1 year ago

  • % Done changed from 0 to 90

Related issue (broken fromWkb): #14182

Updated by Sandro Santilli over 1 year ago

  • Status changed from In Progress to Closed

Updated by Sandro Santilli over 1 year ago

  • Status changed from Closed to Reopened
  • Target version set to Version 2.14

I'm reopening this to signal the fix did not include an automated testcase
(I suggest we add a "ready for test" status)

Updated by Sandro Santilli about 1 year ago

Test file stubbed in https://github.com/qgis/QGIS/pull/2744 -- no specific test for this case yet (needs to fix #14182 first, or I'll find another workaround via friendship)

Updated by Sandro Santilli about 1 year ago

Finally handled to produce the automated testcase for this special WKB: https://github.com/qgis/QGIS/pull/2750

The test checks that simplification cannot happen, due to the supposedly "malformed" WKB.
But truth is that the WKB is not necessarely malformed and the simplifier code might be improved to handle the special case (of dimension mismatch).
Only, for the sake of this ticket, I think we must be happy with what we have now, as the whole simplification code might be replaced anyway to somethign that doesn't act directly on the WKB (recommended).

PR https://github.com/qgis/QGIS/pull/2483 might go in that direction

Updated by Sandro Santilli about 1 year ago

  • Status changed from Reopened to Closed
  • % Done changed from 90 to 100
  • Resolution set to fixed/implemented

Test is in place, closing this as fixed and guard-tested.

Also available in: Atom