Opened 12 years ago

Closed 7 years ago

#3309 closed defect (fixed)

Change in libgeotiff broke MrSID support

Reported by: Even Rouault Owned by: warmerdam
Priority: high Milestone:
Component: GDAL_Raster Version: unspecified
Severity: major Keywords: libgeotiff mrsid gtiff
Cc: hobu, kirk

Description

I have fought the whole day long trying to understand why I got constant crashes when reading .sid files (environmenet : Fedora 12 32 bit, GCC 4.4.2, Geo_DSDK-7.0.0.2167, internal libtiff & libgeotiff), for example on autotest/gdrivers/data/mercator.sid

It finally turns out that the addition of the DefnSet? field in GTIFDefn structure (geo_normalize.h) was the cause (http://trac.osgeo.org/geotiff/changeset/1641 and refreshed from upstream libgeotiff in r18007). This is a change in the ABI. From the symbols needed by libltidsdk.a, I see that GTIFGetDefn() is used. So, as a variable of type struct GTIFDefn can only be allocated by the caller, the addition of the DefnSet? field likely causes some internal state of the mrsid binary to be overwritten which yields to the crash.

So what are the options ? Reverting the addition of DefnSet? field ? In any event, for the future, it might be sane to add an allocator/deallocator for the GTifDefn structure and urge people to use it if we might add new fields.

Change History (12)

comment:1 Changed 12 years ago by warmerdam

I agree that libgeotiff 1.3.0 (and the internal version) are ABI incompatible with what the mrsid sdk expects. I *think* this is not a problem on windows where the libgeotiff presumably included in the dsdk should be used by it.

I have clarified the abi change in 1.3.0 within libgeotiff (http://trac.osgeo.org/geotiff/ticket/19).

I do not see this ticket as a blocker, nor do I see any further action other than making the issue clear for those building mrsid support.

comment:2 Changed 12 years ago by Even Rouault

Resolution: fixed
Status: newclosed

r18431 /trunk/gdal/NEWS: Add a warning about incompatibility between MrSID GeoDSDK and libgeotiff 1.3.0

http://trac.osgeo.org/gdal/wiki/MrSID?action=diff&version=7 : Add note for MrSID and ABI incompatibilities with libgeotiff 1.3.0 or …

comment:3 Changed 12 years ago by mrosen

LT is tracking the need for an updated MrSID SDK internally. The defect number is 10663. I don't have a targeted release date for the community at this time.

comment:4 Changed 11 years ago by kmckelvey

We have addressed this internally for our next release of the MrSID SDK, however any future changes to the GTIFDefn struct will result in similar failure. The problem is that we allocate GTIFDefn (or put it on the stack). We should be letting libgeotiff create this for us but I could not find an interface for it. We need a GTIFCreateGTIFDefn() or something. Should I open a feature request or does this already exist somewhere? I would like to address this permanently if possible.

comment:5 Changed 9 years ago by warmerdam

Priority: highesthigh
Resolution: fixed
Severity: blockermajor
Status: closedreopened

Reopening as there is now a problem with the newer version of libgeotiff with TOWGS84 parameters in the GTIFDefn structure.

comment:6 Changed 9 years ago by warmerdam

Integrate support for libgeotiff 1.4.1 GTIFDefn alloc/free in trunk (r25085).

comment:7 Changed 9 years ago by warmerdam

Integrate support for GEO_NORMALIZE_DISABLE_TOWGS84 libgeotiff directive for binary compatability in trunk (r25086) and 1.9 branch (r25087).

comment:8 Changed 9 years ago by warmerdam

Implied in the above, I have made the following changes in libgeotiff for 1.4.1 release.

2012-10-08 Frank Warmerdam <warmerdam@…>

  • geo_normalize.c/geo_normalize.h: Add support for disabling the TOWGS84 parameter support if GEO_NORMALIZE_DISABLE_TOWGS84 is defined. This is primary intended to maintain binary compatability of the GTIFDefn structure with older versions (for MrSID compat for instance). No configure support - you need to manually incorporate into geo_config.h or perhaps geo_normalize.h. (gdal #3309)
  • geo_normalize.c/geo_normalize.h: Add GTIFAllocDefn() and GTIFFreeDefn() functions for allocation/free of GTIF structure in a more version independent way.

The GEO_NORMALIZE_DISABLE_TOWGS84 change should make it possible to build libgeotiff in a way that is binary compatability with MrSID SDK (such as DSDK 7.0.0).

comment:9 Changed 9 years ago by warmerdam

Component: defaultGDAL_Raster
Keywords: gtiff added

comment:10 Changed 9 years ago by kirk

Cc: kirk added

comment:11 Changed 7 years ago by Jukka Rahkonen

Would be nice to know if this ticket is ready for closing.

comment:12 Changed 7 years ago by Even Rouault

Resolution: fixed
Status: reopenedclosed
Note: See TracTickets for help on using tickets.