INGR causes a bus error on Solaris 64-bit with Sun Studio 11

[tclarke]

Solaris 10 sparc 64-bit Sun Studio 11 builds of the INGR driver cause a BUS exception due to an alignment issue in the INGR driver. I'm attaching a patch which fixes the crash.

[tclarke]

Patch which fixes the crash

[warmerdam]

Ivan,

Can you review and apply as appropriate. We should likely take this back into the 1.5 branch too (for 1.5.4).

[Even Rouault]

If I can share my opinion, I think the proposed fix is only a workaround that prevents unaligned memory access but that's not the correct fix. I'd bet it will certainly fail to read any valid INGR dataset.

#pragma pack(8) will change the size of structures like the header (INGR_HeaderOne, INGR_HeaderTwoA, ...) which are in fact the exact layouts of the header on file when #pragma pack(1) is set.

The current code of the INGR driver should be changed to not directly fread in a single time the structure and access their fields. It should rather be something like :

VSIFReadL(&pHeaderOne->WordToFollow, 1, sizeof(short), fp); VSIFReadL(&pHeaderOne->DataTypeCode, 1, sizeof(short), fp); etc...

Same thing for the write part of course. Not a one-liner kind of fix obviously.

In a more general way, we should ban the use #pragma pack for any portable code on architectures not supported unaligned access. A quick scan of the gdal source code shows another use in the MSGN driver (in msg_basic_types.h).

[warmerdam]

I would generally concur with Even's analysis. Depending on structure packing to match layouts of stuff from disk is very risky, and impedes portability.

[ilucena]

I completely agree. I should have loaded the records it on a buffer and used memcpy() field by field. That also would lead to a better solution to take care of the LSB conversions. The question now is should I do it now since there some other tests to be done on RLE compression or should I apply the workaround and let those change for a later time. Be aware that are a lot of optionals sub-formats [1] supported by this driver and the test are going to be very time consuming.

[1] - Tile and untiled: RLE, RLE Color Table, RLE RGB, Adaptative RGB, CCITT 4, JPEG, Continuous Tone and the regular flat pixel blocks.

[Even Rouault]

Workaround patch commited in r15529.

[ilucena]

I have been working on a solution to eliminate the pragma declaration. So far, the tests are proven the changes to be successful and I will commit it shortly. Thanks for the help and advices.

[ilucena]

Revision 15559 applied to trunk.

Loads and unloads struct to buffer and buffer to struct:

http://trac.osgeo.org/gdal/browser/trunk/gdal/frmts/ingr/IngrTypes.cpp#L1081

based on macro:

http://trac.osgeo.org/gdal/browser/trunk/gdal/frmts/ingr/IngrTypes.h#L539

[Even Rouault]

Fix for compilation on big-endian host commited in r15563