shp2pgsql is sometimes missing the INSERT statements

[myon]

I've been chasing regression tests failures seen in 3.2 on Debian unstable: ​https://pgdgbuild.dus.dg-i.net/view/Binaries/job/postgis-binaries/326/architecture=amd64,distribution=sid/console The sympton is always that the diff shows some rows missing.

I managed to track it down to shp2pgsql sometimes missing the INSERT part; running it in a loop will eventually trigger the bug.

while ./loader/shp2pgsql -G  -g the_geom ./regress/loader/MultiPointZ.shp loadedshp | grep INSERT; do :; done
...
Shapefile type: MultiPointZ
Postgis type: MULTIPOINT[4]
INSERT INTO "loadedshp" (the_geom) VALUES ('01040000E0E61000000300000001010000C00000000000000000000000000000F03F0000000000000040000000000000084001010000C00000000000002240000000000000F0BF00000000000000C000000000000008C001010000C00000000000002240000000000000F0BF00000000000034C00000000000C05EC0');
Shapefile type: MultiPointZ
Postgis type: MULTIPOINT[4]
INSERT INTO "loadedshp" (the_geom) VALUES ('01040000E0E61000000300000001010000C00000000000000000000000000000F03F0000000000000040000000000000084001010000C00000000000002240000000000000F0BF00000000000000C000000000000008C001010000C00000000000002240000000000000F0BF00000000000034C00000000000C05EC0');
Shapefile type: MultiPointZ
Postgis type: MULTIPOINT[4]
  <-- no INSERT here

[robe]

Myon noted he can trigger in 3.1.4 but not 3.1.3

Note from IRC:

Myon
oookay, I can trigger the bug using shp2pgsql from 3.1.4+dfsg-1.pgdg+1 on unstable
but not using 3.1.3+dfsg-1~exp1.pgdg+1+b1 on (a semi-up-to-date) bookworm

So this could be a debian unstable issue though strk recalls being able to trigger these things — but we know strk doesn't ever run bleeding edge OS.

[myon]

Thanks robe2 and strk on #postgis for their patience. More datapoints:

15 22:06 <Myon> oookay, I can trigger the bug using shp2pgsql from 3.1.4+dfsg-1.pgdg+1 on unstable
15 22:07 <Myon> but not using 3.1.3+dfsg-1~exp1.pgdg+1+b1 on (a semi-up-to-date) bookworm
15 22:13 <Myon> 3.1.1+dfsg-1 on bullseye seems ok
15 22:14 <Myon> 3.1.4+dfsg-1.pgdg110+1 on bullseye seems ok
15 22:18 <Myon> 3.1.4+dfsg-1.pgdg+1 on bookworm seems ok
15 22:19 <Myon> 3.2.0~alpha1+dfsg-1~exp1 from experimental installed on bookworm is also ok
15 22:19 <Myon> so it seems it might be an issue with the OS or some library on unstable
15 22:19 <Myon> ... that hasn't trickled down to testing/bookworm yet

[robe]

Additional notes on #4995

[strk]

I can reproduce on sid chroot. Valgrind says:

==38338== Conditional jump or move depends on uninitialised value(s)
==38338==    at 0x1175B6: ShpLoaderGenerateSQLRowStatement (shp2pgsql-core.c:1564)
==38338==    by 0x10BDF5: main (shp2pgsql-cli.c:397)

[strk]

So if my reading is correct, the INSERT is missing when the uninitialized memory contains bits that make shp2pgsql think the record was deleted.

[strk]

The valgrind report also comes up on Ubuntu 20.10

[strk]

Still present, the valgrind reported error, on 64bit and with reduced switches:

$ valgrind ./loader/shp2pgsql  ./regress/loader/PointZ.shp 
...
Shapefile type: PointZ
Postgis type: POINT[4]
==49529== Conditional jump or move depends on uninitialised value(s)
==49529==    at 0x119ED1: ShpLoaderGenerateSQLRowStatement (shp2pgsql-core.c:1564)
==49529==    by 0x10E6A8: main (shp2pgsql-cli.c:397)
...

[strk]

It looks like updating dbfopen.c to current master branch of ​https://github.com/OSGeo/shapelib fixes the issue.

[strk]

Actually I'm wrong: using the latest dbfopen.c does fix the valgrind error but results in no records being used.

Rerting commit [595f840ebc479657adac9143cfaf890faba72f99/git] instead fixes the valgrind reported error while still passing testsuite. Paul: that commit was yours

Note that when reverting the above commit we get on stderr this printed: fread(1) failed on DBF file

[strk]

Paul: reverting commit [595f840ebc479657adac9143cfaf890faba72f99/git] still passes testsuite, do you remember HOW you could trigger the misbehaviour which made you push that change ? Is it not in the testsuite ?

[strk]

This seems relevant: ​https://github.com/OSGeo/shapelib/pull/4

[Sandro Santilli <strk@…>]

In 7b3f100/git:

Revert "Harmonize with old behaviour."

This reverts commit 595f840ebc479657adac9143cfaf890faba72f99.

Fixes #4994

[myon]

Fwiw this needs backporting, the problem is at least also visible using shp2pgsql from 3.1.4+dfsg-1.pgdg+1 on Debian unstable.

[Sandro Santilli <strk@…>]

In 7b58a46/git:

Revert "Harmonize with old behaviour."

This reverts commit 595f840ebc479657adac9143cfaf890faba72f99.

Fixes #4994 in 3.1 branch (3.1.5dev)