db/drivers/dbf fail to build due to undefined reference to 'SASetupDefaultHooks' (SHAPELIB)

[marisn]

Revision: 30825 OS: Ubuntu 7.10 64bit

gcc -L/home/dace/soft/grass_trunk/dist.x86_64-unknown-linux-gnu/lib -Wl,--export-dynamic -Wl,-rpath-link,/home/dace/soft/grass_trunk/dist.x86_64-unknown-linux-gnu/lib -o /home/dace/soft/grass_trunk/dist.x86_64-unknown-linux-gnu/driver/db/dbf OBJ.x86_64-unknown-linux-gnu/column.o OBJ.x86_64-unknown-linux-gnu/cursor.o OBJ.x86_64-unknown-linux-gnu/db.o OBJ.x86_64-unknown-linux-gnu/dbfexe.o OBJ.x86_64-unknown-linux-gnu/describe.o OBJ.x86_64-unknown-linux-gnu/driver.o OBJ.x86_64-unknown-linux-gnu/error.o OBJ.x86_64-unknown-linux-gnu/execute.o OBJ.x86_64-unknown-linux-gnu/fetch.o OBJ.x86_64-unknown-linux-gnu/listtab.o OBJ.x86_64-unknown-linux-gnu/main.o OBJ.x86_64-unknown-linux-gnu/select.o OBJ.x86_64-unknown-linux-gnu/str.o OBJ.x86_64-unknown-linux-gnu/table.o OBJ.x86_64-unknown-linux-gnu/create_table.o -lgrass_dbmidriver -lgrass_dbmibase -lgrass_gis -lgrass_datetime -lz -lgrass_dbstubs -lgrass_dbmibase -lgrass_gis -lgrass_datetime -lz -lgrass_gis -lgrass_datetime -lz -lgrass_gis -lgrass_datetime -lz -lgrass_dbmibase -lgrass_gis -lgrass_datetime -lz -lgrass_sqlp -lgrass_shape -lgrass_dbstubs -lgrass_dbmibase -lgrass_gis -lgrass_datetime -lz -lgrass_gis -lgrass_datetime -lz -lgrass_gis -lgrass_datetime -lz -lgrass_datetime -L/usr/lib -lgdal1.4.0 -lm -lz /home/dace/soft/grass_trunk/dist.x86_64-unknown-linux-gnu/lib/libgrass_shape.so: undefined reference to `SASetupDefaultHooks' collect2: ld returned 1 exit status make: *** /home/dace/soft/grass_trunk/dist.x86_64-unknown-linux-gnu/driver/db/dbf Error 1

[marisn]

On 32bit Gentoo box with gdal-1.5.0 GRASS trunk rev. 30831 compiles just fine, but any module, that uses dbf, fails with error: dbmi: Protocol error WARNING:Cannot open select cursor

(gdb) run -a map=geology@PERMANENT east_north=592948.879668,4927119.3361 distance=296.261682243
Starting program: /home/ddd/soft/grass_trunk/dist.i686-pc-linux-gnu/bin/v.what -a map=geology@PERMANENT east_north=592948.879668,4927119.3361 distance=296.261682243
[Thread debugging using libthread_db enabled]
[New Thread 0xb751e6d0 (LWP 17661)]

East: 592948.879668
North: 4927119.3361

Map: geology
Mapset: PERMANENT
Objekta tips: Laukums
Kavadrātmetri: 10056250.000
Hektāri: 1005.625
Akri: 2484.893
Kvadrātjūdzes: 3.8826
Līmenis: 1
Kategorija: 6

Driver: dbf
Database: /home/ddd/hdo/maris/gis_dati/spearfish60/PERMANENT/dbf/
Table: geology
Key column: cat
dbmi: Protocol error
BRĪDINĀJUMS:Cannot open select cursor

Program received signal SIGPIPE, Broken pipe.
[Switching to Thread 0xb751e6d0 (LWP 17661)]
0xb7fb0410 in __kernel_vsyscall ()
(gdb) bt
#0  0xb7fb0410 in __kernel_vsyscall ()
#1  0xb79b7453 in write () from /lib/libc.so.6
#2  0xb79651d7 in _IO_file_write () from /lib/libc.so.6
#3  0xb7964e85 in ?? () from /lib/libc.so.6
#4  0xb7965136 in _IO_file_xsputn () from /lib/libc.so.6
#5  0xb795b19f in fwrite () from /lib/libc.so.6
#6  0xb7f77f4b in db__send (buf=0xbfe42d70, size=4) at xdr.c:80
#7  0xb7f78c55 in db__send_int (n=101) at xdrint.c:9
#8  0xb7f78d22 in db__start_procedure_call (procnum=101) at xdrprocedure.c:11
#9  0xb7edc3b9 in db_close_database (driver=0x80e5730) at c_closedb.c:17
#10 0xb7a5d948 in F_generate (drvname=0x80e3e18 "dbf",
    dbname=0x809ec88 "/home/ddd/hdo/maris/gis_dati/spearfish60/PERMANENT/dbf/", tblname=0x80e3df8 "geology",
    key=0x80e3e08 "cat", keyval=6, frmname=0x0, frmmapset=0x0, edit_mode=1, format=2, form=0xbfe447dc)
    at generate.c:88
#11 0x0804a864 in what (east=592948.87966800004, north=4927119.3361, maxdist=296.261682243, width=7, mwidth=9,
    topo=0, showextra=1) at what.c:276
#12 0x08049ae4 in main (argc=5, argv=0xbfe42db0) at main.c:175

[kyngchaos]

Looking at the GDAL copy of the shapelib, I see that SASetupDefaultHooks is in a new file: shp_vsi.c. This should probably be added to GRASS' copy.

[kyngchaos]

Ack, shp_vsi.c is a GDAL thing - it's an interface between shapelib and GDAL's VSI service. Maybe we need to go back to the previous shapelib?

[neteler]

Doesn't  http://trac.osgeo.org/grass/changeset/30820 fix the problem? It did so for me. I think that we should fix it instead of reverting to the older and buggy SHAPELIB. The last years it was always in sync.

Markus

PS: I cc to Frank for further insights.

[hamish]

Any clarification on if it will work with GDAL < 1.5.0? (I can test that, later)

Hamish

[neteler]

Question to Frank: Should we stop syncing SHAPELIB from GDAL? I dunno. At least it will become hard to backport SHAPELIB fixes in future.

Comment from Glynn:

On Mon, Mar 31, 2008 at 2:26 PM, Glynn Clements <glynn@…> wrote: ...

They appear to be part of GDAL, so GDAL is now a dependency of the DBF driver:

...

Markus: is this change really necessary? Specifically, is the GDAL-isation actually necessary? AFAICT, apart from some sanity checks in SHPReadObject(), the change is almost entirely down to using GDAL's custom I/O interface. I would have thought it would be better to just add the sanity checks and do without GDAL's I/O interface, particularly as the code can only ever use the default hooks (which presumably are just the stdio function).

[glynn]

Replying to neteler:

Doesn't  http://trac.osgeo.org/grass/changeset/30820 fix the problem? It did so for me.

The new code only works with very recent versions of GDAL.

I think that we should fix it instead of reverting to the older and buggy SHAPELIB. The last years it was always in sync.

By all means sync the bugfixes, but not the gdal-isation.

[rouault]

kyngchaos gave the solution.

All you need to do is to replace the implementation of shp_vsi.c that is done in GDAL by the one which is most appropriate to GRASS if there is an abstraction I/O layer in GRASS (I don't know GRASS). One of the solutions is just to do a trivial implementation thats maps the VSI_SHP_Open /VSI_SHP_Seek/VSI_SHP_Write/VSI_SHP_Read/VSI_SHP_Flush/VSI_SHP_Tell_VSI_SHP_Close to fopen/fseek/fwrite/fread/fflush/ftell/fclose.

And you should be able to continue to sync dbfopen.c, shpopen.c and shapefil.h

[neteler]

As suggested by Frank, I have added safileio.c:

 http://trac.osgeo.org/grass/changeset/30934

If I now remove the GDAL dependency from db/driver/dbf/Makefile it does not compile. If I keep it, it compiles. Problem solved?

Markus

On Wed, Apr 9, 2008 at 7:01 PM, Frank Warmerdam <warmerdam pobox.com> wrote:

Folks, I'm just trying to catch up on this topic. There is already a stdio based implementation in shapelib itself. I have attached it,and it is normally in a file called safileio.c. I'm afraid I'm uncertain of the implications of mixing the Shapelib 1.3 code with GDAL's using Shapelib 1.2. It seems potentially like it could be a problem.

[neteler]

Frank,

a question remains: how to get updates for safileio.c?

Markus

[martinl]

Replying to neteler:

If I now remove the GDAL dependency from db/driver/dbf/Makefile it does not compile. If I keep it, it compiles. Problem solved?

well, it is possible now to compile shapelib and dbf driver. Copying vector map using dbf driver fails

GRASS 6.3.svn (spearfish60):~/smetiste/grass_trunk > g.copy vect=soils,s
Copy vector <soils@PERMANENT> to current mapset as <s>
dbmi: Protocol error
WARNING: Cannot open select cursor: 'select * from soils where 0 = 1'

Martin

[neteler]

Even R. suggests: if running Linux i386 or amd64, 'valgrind --trace-children=yes db.select xxxxx' will report memory errors on all forked processes. Currently discussed in #gdal (IRC)

[dylan]

Replying to neteler:

Even R. suggests: if running Linux i386 or amd64, 'valgrind --trace-children=yes db.select xxxxx' will report memory errors on all forked processes. Currently discussed in #gdal (IRC)

Here is my output (Debian/Unstable? x86 32-bit)

valgrind --trace-children=yes db.select bugsites
==9294== Memcheck, a memory error detector.
==9294== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==9294== Using LibVEX rev 1804, a library for dynamic binary translation.
==9294== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==9294== Using valgrind-3.3.0-Debian, a dynamic binary instrumentation framework.
==9294== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==9294== For more details, rerun with: -v
==9294== 
==9295== Memcheck, a memory error detector.
==9295== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==9295== Using LibVEX rev 1804, a library for dynamic binary translation.
==9295== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==9295== Using valgrind-3.3.0-Debian, a dynamic binary instrumentation framework.
==9295== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==9295== For more details, rerun with: -v
==9295== 
==9295== Memcheck, a memory error detector.
==9295== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==9295== Using LibVEX rev 1804, a library for dynamic binary translation.
==9295== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==9295== Using valgrind-3.3.0-Debian, a dynamic binary instrumentation framework.
==9295== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==9295== For more details, rerun with: -v
==9295== 
==9295== Use of uninitialised value of size 4
==9295==    at 0x44F6F4C: VSIFReadL (cpl_vsil.cpp:423)
==9295==    by 0x461EAA1: VSI_SHP_Read (shp_vsi.c:53)
==9295==    by 0x40EC994: DBFOpenLL (in /usr/local/grass-6.3.svn/lib/libgrass_shape.6.3.svn.so)
==9295==    by 0x40EC5A9: DBFOpen (in /usr/local/grass-6.3.svn/lib/libgrass_shape.6.3.svn.so)
==9295==    by 0x804F237: load_table_head (in /usr/local/grass-6.3.svn/driver/db/dbf)
==9295==    by 0x804BA51: execute (in /usr/local/grass-6.3.svn/driver/db/dbf)
==9295==    by 0x804EC11: db__driver_open_select_cursor (in /usr/local/grass-6.3.svn/driver/db/dbf)
==9295==    by 0x402B82D: db_d_open_select_cursor (in /usr/local/grass-6.3.svn/lib/libgrass_dbmidriver.6.3.svn.so)
==9295==    by 0x402BE9D: db_driver (in /usr/local/grass-6.3.svn/lib/libgrass_dbmidriver.6.3.svn.so)
==9295==    by 0x804EBAB: main (in /usr/local/grass-6.3.svn/driver/db/dbf)
==9295== 
==9295== Invalid read of size 4
==9295==    at 0x44F6F63: VSIFReadL (cpl_vsil.cpp:423)
==9295==    by 0x461EAA1: VSI_SHP_Read (shp_vsi.c:53)
==9295==    by 0x40EC994: DBFOpenLL (in /usr/local/grass-6.3.svn/lib/libgrass_shape.6.3.svn.so)
==9295==    by 0x40EC5A9: DBFOpen (in /usr/local/grass-6.3.svn/lib/libgrass_shape.6.3.svn.so)
==9295==    by 0x804F237: load_table_head (in /usr/local/grass-6.3.svn/driver/db/dbf)
==9295==    by 0x804BA51: execute (in /usr/local/grass-6.3.svn/driver/db/dbf)
==9295==    by 0x804EC11: db__driver_open_select_cursor (in /usr/local/grass-6.3.svn/driver/db/dbf)
==9295==    by 0x402B82D: db_d_open_select_cursor (in /usr/local/grass-6.3.svn/lib/libgrass_dbmidriver.6.3.svn.so)
==9295==    by 0x402BE9D: db_driver (in /usr/local/grass-6.3.svn/lib/libgrass_dbmidriver.6.3.svn.so)
==9295==    by 0x804EBAB: main (in /usr/local/grass-6.3.svn/driver/db/dbf)
==9295==  Address 0x4f46424c is not stack'd, malloc'd or (recently) free'd
==9295== 
==9295== Process terminating with default action of signal 11 (SIGSEGV)
==9295==  Access not within mapped region at address 0x4F46424C
==9295==    at 0x44F6F63: VSIFReadL (cpl_vsil.cpp:423)
==9295==    by 0x461EAA1: VSI_SHP_Read (shp_vsi.c:53)
==9295==    by 0x40EC994: DBFOpenLL (in /usr/local/grass-6.3.svn/lib/libgrass_shape.6.3.svn.so)
==9295==    by 0x40EC5A9: DBFOpen (in /usr/local/grass-6.3.svn/lib/libgrass_shape.6.3.svn.so)
==9295==    by 0x804F237: load_table_head (in /usr/local/grass-6.3.svn/driver/db/dbf)
==9295==    by 0x804BA51: execute (in /usr/local/grass-6.3.svn/driver/db/dbf)
==9295==    by 0x804EC11: db__driver_open_select_cursor (in /usr/local/grass-6.3.svn/driver/db/dbf)
==9295==    by 0x402B82D: db_d_open_select_cursor (in /usr/local/grass-6.3.svn/lib/libgrass_dbmidriver.6.3.svn.so)
==9295==    by 0x402BE9D: db_driver (in /usr/local/grass-6.3.svn/lib/libgrass_dbmidriver.6.3.svn.so)
==9295==    by 0x804EBAB: main (in /usr/local/grass-6.3.svn/driver/db/dbf)
==9295== 
==9295== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 109 from 1)
==9295== malloc/free: in use at exit: 84,377 bytes in 52 blocks.
==9295== malloc/free: 82 allocs, 30 frees, 121,294 bytes allocated.
==9295== For counts of detected errors, rerun with: -v
==9295== searching for pointers to 52 not-freed blocks.
==9295== checked 3,163,220 bytes.
==9295== 
==9295== LEAK SUMMARY:
==9295==    definitely lost: 32 bytes in 2 blocks.
==9295==      possibly lost: 67 bytes in 2 blocks.
==9295==    still reachable: 84,278 bytes in 48 blocks.
==9295==         suppressed: 0 bytes in 0 blocks.
==9295== Rerun with --leak-check=full to see details of leaked memory.
dbmi: Protocol error
==9294== 
==9294== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 23 from 1)
==9294== malloc/free: in use at exit: 6,881 bytes in 74 blocks.
==9294== malloc/free: 148 allocs, 74 frees, 276,120 bytes allocated.
==9294== For counts of detected errors, rerun with: -v
==9294== searching for pointers to 74 not-freed blocks.
==9294== checked 118,464 bytes.
==9294== 
==9294== LEAK SUMMARY:
==9294==    definitely lost: 3,949 bytes in 8 blocks.
==9294==      possibly lost: 0 bytes in 0 blocks.
==9294==    still reachable: 2,932 bytes in 66 blocks.
==9294==         suppressed: 0 bytes in 0 blocks.
==9294== Rerun with --leak-check=full to see details of leaked memory.

[dylan]

with GRASS (r30956) and GDAL (r14266) I no longer get DBMI errors.

[rouault]

In the Valgrind log, it's clear that it was the GDAL's SASetupDefaultHooks version that was used. I guess that if things are right now with GRASS r30956, it's because safileio.c has been added to the GRASS tree now, and that its SASetupDefaultHooks version that is used now.

However, it's really risky to have a library 'A', namely GRASS, that contains SHAPELIB symbols and that links with another library 'B', namely GDAL, that also contains SHAPELIB symbols... I'm not sure there's a guarantee that the symbols that will be used will come entirely from library 'A' or entirely from library 'B'. I'd be afraid that there's a mix... And here, you really want that the GRASS DBF driver uses its SHAPELIB symbols, not the GDAL ones.

To be really safe, I would recommend a change to SHAPELIB to add decoration to all its symbols, like that

#ifndef SHAPELIBDECORATE
#define SHAPELIBDECORATE(x) x
#endif


DBFHandle SHPAPI_CALL
      SHAPELIBDECORATE(DBFOpen)( const char * pszDBFFile, const char * pszAccess );
DBFHandle SHPAPI_CALL
      SHAPELIBDECORATE(DBFOpenLL) ( const char * pszDBFFile, const char * pszAccess,
                 SAHooks *psHooks );
DBFHandle SHPAPI_CALL
      SHAPELIBDECORATE(DBFCreate) ( const char * pszDBFFile );
DBFHandle SHPAPI_CALL
      SHAPELIBDECORATE(DBFCreateLL)( const char * pszDBFFile, SAHooks *psHooks );

int	SHPAPI_CALL
      SHAPELIBDECORATE(DBFGetFieldCount)( DBFHandle psDBF );
int	SHPAPI_CALL
      SHAPELIBDECORATE(DBFGetRecordCount) ( DBFHandle psDBF );
int	SHPAPI_CALL
      SHAPELIBDECORATE(DBFAddField) ( DBFHandle hDBF, const char * pszFieldName,
                   DBFFieldType eType, int nWidth, int nDecimals );

...

The shapelib included within GDAL would be compiled with :

#define SHAPELIBDECORATE(x) GDAL##x

The shapelib included within GRASS would be compiled with :

#define SHAPELIBDECORATE(x) GRASS##x

That's a rather heavy and boring change to SHAPELIB headers and sources, and to GRASS and GDAL source code, but that would solve definitely the problems.

(Side node, we should do the samething in GDAL with libtiff...)

[warmerdam]

I'm not keen on applying the decoration idea in GDAL or shapelib upstream. But manually renaming the functions in the copy kept in GRASS would avoid any problems.

Alternatively we could treat shapelib more as an external dependency (at least optionally) for GDAL and GRASS so one copy can be easily shared.

Potentially GRASS could also just depend on GDAL for it's shapelib instead of having an embedded copy.

[neteler]

[please respond in trac, grass-dev cannot post to trac]

On Sun, Apr 13, 2008 at 1:54 AM, Glynn Clements <glynn@…> wrote:

Comment (by rouault): In the Valgrind log, it's clear that it was the GDAL's SASetupDefaultHooks version that was used. I guess that if things are right now with GRASS r30956, it's because safileio.c has been added to the GRASS tree now, and that its SASetupDefaultHooks version that is used now. However, it's really risky to have a library 'A', namely GRASS, that contains SHAPELIB symbols and that links with another library 'B', namely GDAL, that also contains SHAPELIB symbols...

The DBF driver isn't linked against GDAL any more, so this shouldn't be an issue. There could be a problem if you use shapelib in some other library or executable which *is* linked against GDAL but, right now, shapelib is only used by the DBF driver.

[neteler]

Maris, William, is the problem solved (for now)?

Question: Can we remove the Shapelib copy and use only GDAL/OGR for this? The needed functions are

nm /home/neteler/grass63/dist.x86_64-unknown-linux-gnu/driver/db/dbf | grep DBF
                 U DBFAddField
                 U DBFClose
                 U DBFCreate
                 U DBFGetFieldCount
                 U DBFGetFieldInfo
                 U DBFGetRecordCount
                 U DBFIsAttributeNULL
                 U DBFOpen
                 U DBFReadDoubleAttribute
                 U DBFReadIntegerAttribute
                 U DBFReadStringAttribute
                 U DBFWriteDoubleAttribute
                 U DBFWriteIntegerAttribute
                 U DBFWriteNULLAttribute
                 U DBFWriteStringAttribute

In GDAL/OGR, the AVC driver uses DBF bindings:

gdal/ogr/ogrsf_frmts/avc/

There is a local copy of dbfopen.h. Perhaps we can do the same?

Markus

[marisn]

Seems to be fixed for now. Tested on 32bit Gentoo with GDAL-1.5.0 and GRASS trunk rev. 31009