Ok, I'm forking a new issue off issue842

Doing something like

 for i in _darcs/patches/*.gz; do zcat $i > /dev/null; done 2> crc-errors

Reveals the following errors.

Is it worthwhile for us to pursue this, i.e. try and figure out (a) how such a
thing could have happened and (b) if it's bad for darcs in any way?

Attachments

• crc-errors-kowey

I just tried this on the maybench repository and got similar CRC check errors.
  
Next, we'll need to see if we can generate such errors on a purely local
repository (i.e. no fetch errors) to confirm that it's something that darcs is
doing.

Odd stuff.  Darcs just uses libz for all its zipping.

Progress! We can now reproduce this error without any network intervention
whatsoever.

To create a repository with CRC errors, you can just run ./darcs-benchmark
(cabal install maybench) and look in /tmp/darcs-benchmark/main.  This just needs
to be boiled down to a shell script so we get a clearer idea what's up.

Attached is a minimal test case for this bug using only one patch (which I have
also sent as a patch)

I'll note that you can trigger this either by applying a minimal bundle of this
sort or by doing a darcs pull.

Also, gzip --test is a useful way to do these tests.  No silly for loops with zcat.

Attachments

• issue844_gzip_crc.sh

Not sure if it helps, but:

Upon closer inspection of the test case, I see that just the CRC
differs, this is the xxd dump of the _darcs/patches/...*.gz files.

I wonder if there's a confusion of adler32 vs. crc32 checksums due to
inconsistent/missing initialization, threaded use of non-threadsafe
functions, or something, since the stream itself remains the same.
See /usr/include/zlib.h for details.

Note I have zero clue of Haskell beyond that fact that it is supposed to
be a functional language...

(in bash):

diff -U4 \
<(xxd temp1/_darcs/patches/20081110162047-7b960-924b1d05206f7d4c3450a8a4d2ee822302b4dd1e.gz) \
<(xxd temp2/_darcs/patches/20081110162047-7b960-924b1d05206f7d4c3450a8a4d2ee822302b4dd1e.gz)
--- /dev/fd/63  2008-11-10 17:28:29.238323536 +0100
+++ /dev/fd/62  2008-11-10 17:28:29.241324386 +0100
@@ -1,5 +1,5 @@
 0000000: 1f8b 0800 0000 0000 0003 8bce cccb 2ce1  ..............,.
 0000010: cad5 4d74 48cf add0 4b49 d5d2 3232 30b0  ..MtH...KI..220.
 0000020: 3034 3434 3034 3332 3031 8f55 a8e6 4a4c  0444043201.U..JL
 0000030: 4949 cbcc 4955 d0d3 4fe4 ca28 cdcb 0631  II..IU..O..(...1
-0000040: 140c b9b4 b96a b900 aae0 80fc 3f00 0000  .....j......?...
+0000040: 140c b9b4 b96a b900 61b2 9288 3f00 0000  .....j..a...?...

And the differing bytes are indeed the CRC32 (the crc is in little
endian format in the file...)

$ gzip -vl temp?/_darcs/patches/200811101620*
method  crc     date  time  compressed uncompressed ratio uncompressed_na...
defla fc80e0aa Nov 10 17:20         80           63 1.6%  temp1/_darcs/pa...
defla 8892b261 Nov 10 17:20         80           63 1.6%  temp2/_darcs/pa...

It appears that using the Haskell zlib 0.5.0.0 fixes this
(Darcs 2.1.0 from DARCS repo, GHC 6.8.3 on openSUSE 11.0 i386).

Are there issues with the C <-> Haskell interfacing of c-zlib?

BTW, when using darcs that has been compiled to use the Haskell zlib
(rather than C zlib), you cannot pull from a repository that has borked
CRCs: "darcs failed:  Codec.Compression.Zlib: incorrect data check"

I think we should treat this as something relatively urgent.

It sounds like we need to figure out (1) what people can do to recover from
this?  Can we give repository owners a dumb shell script to gunzip and regzip
files? (2) figure out why this was breaking and fix it (3) make darcs repair fix
this [as suggested by Christian Kellermann].

We could also use the fact that it only happens with our homegrown bindings and
not the hackage ones as a clue.

Duncan: do you have any advice?

(over to you, Thorkil! :-) )

I notice that issue844_gzip_crc.sh fails to notice any trouble on my system. 
What sorts of systems *does* it cause trouble on?

It sounds a whole lot like it's simply a broken zlib install.

David

David Roundy:

> I notice that issue844_gzip_crc.sh fails to notice any trouble on my system. 

That's because the c-zlib either does not complain about CRC
mismatch and silently ignores the issue, or because the error is
symmetric within the c-zlib bindings in DARCS.

That you cannot reproduce it will hamper debugging, but doesn't disprove
the bug.

> What sorts of systems *does* it cause trouble on?

Mine for one, see issue1215 for further info.

WRT repairing things, I have two suggestions, either one alone is
sufficient:

1. a simple POSIX sh for loop with zcat does the job for me, an initial
   very raw version is:

   gzip -t _darcs/patches/*.gz 2>&1 | grep crc | cut -f2 -d: | \
     while read a ; do zcat <$a >${a%.gz} ; rm $a ; gzip ${a%.gz} ; done

   Note this can corrupt the repo if there are other failures on zcat,
   such as out-of-space, so ONLY do that on a COPY of the repo.

2. alternatively, with a c-zlib version of darcs:

   mkdir foo
   cd foo
   darcs init --hashed
   darcs pull ../repo-with-borked-crc

   For some reason that is beyond me, the --hashed repo does not show
   the corruption, as can be verified by a subsequent

   gzip -vt _darcs/patches/*-*

HTH

Hi Matthias,

Is there any way we could turn this into a reproducible test case (i.e. a shell
script?)

See the bugs/ and tests/ directory (notably the issue844 one that David pointed
to) for examples.

What would be extra-nice is a patch adding such a test case to our bugs directory!

On Mon, Nov 10, 2008 at 08:32:51PM -0000, Matthias Andree wrote:
> > I notice that issue844_gzip_crc.sh fails to notice any trouble on
> > my system.
> 
> That's because the c-zlib either does not complain about CRC
> mismatch and silently ignores the issue, or because the error is
> symmetric within the c-zlib bindings in DARCS.
> 
> That you cannot reproduce it will hamper debugging, but doesn't
> disprove the bug.

That's not true, the issue844_gzip_crc.sh script uses gzip -t to check
for crc mismatches.  I don't disagree that there is a bug, but only
that the bug has been shown to be in darcs' source code.  It could
just as easily be in your zlib itself, and given that the bug doesn't
manifest itself on many computers, that seems likely.  You and Eric
are able to reproduce it, but you haven't shared how you did so.

> > What sorts of systems *does* it cause trouble on?
> 
> Mine for one, see issue1215 for further info.

I see.  It fails on a suse system....  The bug isn't present on my
ubuntu hardy heron system with

zlib: 1.2.3.3
gzip: 1.3.12

(unless there's a problem with the issue844_gzip_crc test script)

> WRT repairing things, I have two suggestions, either one alone is
> sufficient:

I still think the first goal should be to figure out what triggers
this.  It's either something that's unusual about your system and
Eric's, or the issue844_gzip_crc.sh test itself is broken.  Can you
reproduce this issue with that test?

David

Sorry.

Reproduction: I've been using the same script to reproduce the bug.

Zlib the cause: , no matter if I use c-zlib or haskell-zlib, the
executable dynamically links the same file. So why would zlib be the
culprit in this setup? How would I find that out? (I actually tried
ltrace, but this got stuck when darcs - run on behalf of the test script
- asked for my mail address.)

HTH

Interesting. It doesn't fail on Ubuntu Intrepid Ibex (i386) either,
neither on Ubuntu Hardy Heron (i386) for me. Both use GHC 6.8.2 as a
striking difference.

On Mon, 2008-11-10 at 21:02 +0000, Matthias Andree wrote:
> Matthias Andree <matthias.andree@gmx.de> added the comment:
> 
> Sorry.
> 
> Reproduction: I've been using the same script to reproduce the bug.
> 
> Zlib the cause: , no matter if I use c-zlib or haskell-zlib, the
> executable dynamically links the same file. So why would zlib be the
> culprit in this setup?

Though darcs is using different parts of the c-zlib api in the two
cases, the bug could still be in the c-zlib part that implements
gzopen/gzread/gzwrite etc.

So we've established that on the affected machines that the gzwrite
method generates .gz files that gzip thinks have crc errors. I presume
gzread is reading back the files with bad crcs ok or you'd have noticed
this earlier. Do we know exactly what is wrong with the crc on the
affected machines? We know it's wrong but is it the adler32 checksum or
is it just something else wrong entirely?

If it's really that gzwrite is producing borked .gz files then a simple
C test program should be able to demonstrate it.

Do we know the zlib version on the affected machines? The zlib home page
reports the following fix for 1.2.3:
 * Fix crc check bug in gzread() after gzungetc()
Though I note darcs does not use gzungetc().

Note also that the gzip program does not use the zlib shared library, it
has a slightly different internal implementation of the algorithms.

Duncan

Dear zlib maintainers,

I've chosen to Blind-Cc: you on a bug I'm reporting to Darcs's bug
tracking system, http://bugs.darcs.net/issue844/ and I believe we've
encountered a case where zlib's gz*() interface can be made to generate
a file with a bogus checksum that decompresses correctly though. C test
case attached (last attachment in the list).

I've been using zlib 1.2.3 as packaged on openSUSE 11 for i386.

While this message should be self-sufficient as a problem report, in
case the test file attached to this message doesn't provide sufficient
context, you're invited to look at the DARCS bug tracker, URL above.

Dear DARCS BTS users,

On Mon, 10 Nov 2008, Duncan Coutts wrote:

> Though darcs is using different parts of the c-zlib api in the two
> cases, the bug could still be in the c-zlib part that implements
> gzopen/gzread/gzwrite etc.

OK, it might. Apparently, gzwrite() doesn't digest 0-sized writes well.
Details below.

> So we've established that on the affected machines that the gzwrite
> method generates .gz files that gzip thinks have crc errors. I presume
> gzread is reading back the files with bad crcs ok or you'd have noticed
> this earlier. Do we know exactly what is wrong with the crc on the
> affected machines? We know it's wrong but is it the adler32 checksum or
> is it just something else wrong entirely?

No idea yet, and I'm inclined to sidestep this question
given my other findings.

> Note also that the gzip program does not use the zlib shared library, it
> has a slightly different internal implementation of the algorithms.

True, but darcs with Haskell zlib interface also complains when pulling
from repos with broken CRC values.

FWIW, I managed to ltrace the c-zlib interface (darcs 2.0.2,
darcs-b0rked, uses gz* interface) and haskell-zlib interface (darcs
2.1.0 from darcs's darcs repo, darcs-works, uses zCamelCaps interface).

Now, I caught the c-zlib interface doing something like

    gzwrite(gfd, 0, 0);

and this throws zlib 1.2.3 off the track.
Test program (in C) attached, instructions inside. It shows that this
zero-sized write creates a file with broken CRC. Avoiding this 0-sized
write creates a file with intact CRC.

DARCS: darcs should avoid doing zero-sized writes from a NULL pointer
(see ltrace log). I haven't checked if the NULL pointer or the 0 size
are the culprit, not my business - Darcs should be able to avoid this
0-size write.

ZLIB: should handle 0-sized writes more gracefully and not trash the
checksum.

Two darcs ltrace logs, modified test script, C test code attached,
I hope the BTS handles this.

HTH & Best
Matthias

Attachments

• darcs-b0rked
• darcs-works
• issue844_gzip_crc_ma1.sh
• try-zlib.c

I've reported this to Novell (BTS for openSUSE):
https://bugzilla.novell.com/show_bug.cgi?id=443733

On Tue, Nov 11, 2008 at 02:48:29AM -0000, Matthias Andree wrote:
> DARCS: darcs should avoid doing zero-sized writes from a NULL pointer
> (see ltrace log). I haven't checked if the NULL pointer or the 0 size
> are the culprit, not my business - Darcs should be able to avoid this
> 0-size write.
> 
> ZLIB: should handle 0-sized writes more gracefully and not trash the
> checksum.
> 
> Two darcs ltrace logs, modified test script, C test code attached,
> I hope the BTS handles this.

Thanks for tracking this down, Matthias! I've sent in a patch to keep darcs
from calling gzwrite with zero length.

David

Hmm...

seems that there is some point in David's fixing darcs. The bug was just
closed as invalid in the Novell BTS (URL above) with the comment shown below.

I haven't yet heard from the zlib maintainers.

: --- Comment #1 from Vladimir Nadvornik <nadvornik@novell.com> 2008-11-13 08:42:55 MST ---
: gzwrite(foo, 0, 0) seems to correctly return 0 which means error.
: An application should
: 1. not call gzwrite(foo, 0, 0) at all
: 2. check the return value
:
: IMHO it is purely application bug. Please reopen this if zlib
: maintainers think otherwise.

Resolved by
Tue Nov 11 14:51:22 GMT 2008  David Roundy <droundy@darcs.net>
  * resolve issue844: don't call gzwrite with zero length.

I would like to reopen this bug until 'darcs repair' has a way to fix the 
corruption.  It would seem that as more people upgrade darcs they are starting 
to experience this problem.

The script David suggested does seem to be working, but what will windows users 
do?

On Thu, 2008-11-20 at 12:24 +0000, Jason Dagit wrote:
> Jason Dagit <dagit@codersbase.com> added the comment:
> 
> I would like to reopen this bug until 'darcs repair' has a way to fix the 
> corruption.  It would seem that as more people upgrade darcs they are starting 
> to experience this problem.

Note that there is still a bug:

extern int gzclose OF((gzFile file));

/* The return value is the zlib error number
  (see function gzerror below).
 */

But...

foreign import ccall unsafe "static zlib.h gzclose" c_gzclose
    :: Ptr () -> IO ()

Oh no! We've imported it with the wrong type. We're ignoring the error
return. Just like posix close() gzclose() can return an error, but in
the case of zlib it's much more likely, in particular that is when it
reports crc errors.

So that's why darcs has not been noticing these crc errors all this
time. It ignores the error code.

That's also why people only noticed it when using external gzip or once
darcs started optionally using the zlib binding which does check the
error codes.

So once this bug is fixed by checking the error code that gzclose
returns there's still the issue of how to repair old repos that contain
the corruption.

So what is repair going to do? It'll have to explicitly ignore the crc
error (presumably there is some other consistency check you can do for
the case of a genuine crc error). That's not currently possible with the
zlib binding but extending it to report errors explicitly rather than
only via exceptions is on the TODO list for that package.

Duncan

On Thu, 20 Nov 2008, Duncan Coutts wrote:

> So once this bug is fixed by checking the error code that gzclose
> returns there's still the issue of how to repair old repos that contain
> the corruption.

Well, use the broken DARCS version to clone the old repo into a new
--hashed repo, verify that it's OK, and that's it.
(1) The patches/*.gz file content is structured to some extent, so DARCS
should notice if something got corrupted beyond the misled CRC, and
(2) --hashed repos appear unaffected by the gzwrite(_,_,0) bug.

Alternatively, iterate over the .gz files with zcat and repack them,
then check the repo. zcat will write the full file contents before
complaining about the b0rked CRC.

With David's patch,

Tue Nov 11 14:51:22 GMT 2008  David Roundy <droundy@darcs.net>
  * resolve issue844: don't call gzwrite with zero length.

we believe that the cause of these CRC errors have been removed, so I am closing 
this issue as resolved. However, repositories undoubtedly still exist with such 
errors in them. Issue1239 has been created for the task of detecting and 
repairing these errors.

Best regards
Thorkil

I have attached Matthias's suggested fix as a shell script (fix-darcs-crc.sh)

Attachments

• fix-darcs-crc.sh

Eric Kow wrote:

> I have attached Matthias's suggested fix as a shell script (fix-darcs-crc.sh)

Now that's a hack... I'm not sure if that should be blessed as the official
solution - it's ignoring errors besides CRC errors after all.

On Mon, Nov 24, 2008 at 18:47:42 -0000, Matthias Andree wrote:
> > I have attached Matthias's suggested fix as a shell script (fix-darcs-crc.sh)
> 
> Now that's a hack... I'm not sure if that should be blessed as the official
> solution - it's ignoring errors besides CRC errors after all.

Whoops! I meant to say 'workaround' (for the old gzipped files) and not
'fix'.

See the follow-up http://bugs.darcs.net/issue1239

There's some discussion in issue1259 about where the broken CRCs are being produced.

My first thought was that they have been produced for a long time perhaps from
darcs 1.0.9 and beyond.  But on closer inspection, I'm starting to think that
this was from after (as Ian says it likely is)

The evidence:

* darcs 2.0.0 was released on 2008-04-07, and in Zooko's reporting of issue1259,
the earliest modified files were in 2008-04-17.  Perhaps he upgraded eagerly on
that server?

* I tried the issue844 test case on darcs 1.0.9 (disabling the
  optimize --compress step) and it passes

* issue844 was filed on 2008-05, actually after the darcs 2.0.0
  release

What we would need to do now is to compile a copy of darcs 2.0.0,
and try the issue844 test on it; and also darcs 2.0.2

I have tried running tests/issue844_gzip_crc.sh of a fairly recent darcs 
repository with darcs 2.0.0 (unknown) and darcs 2.0.2 (release) and it succeeded 
in both cases. I haven't looked at this test in detail before, however, and I am 
not sure that I am doing the right thing. For which darcs version did this test 
fail?

Thanks and regards
Thorkil

On Mon, Dec 08, 2008 at 19:50:34 -0000, Thorkil Naur wrote:
> I have tried running tests/issue844_gzip_crc.sh of a fairly recent darcs 
> repository with darcs 2.0.0 (unknown) and darcs 2.0.2 (release) and it succeeded 
> in both cases.

Try removing the darcs optimize --compress line in the test case

> I haven't looked at this test in detail before, however, and I am not
> sure that I am doing the right thing. For which darcs version did this
> test fail?

I think the test was introduced in May, a month after the darcs 2
release (but it may also have bitten darcs 2.0.0)

With darcs optimize --compress removed, issue844_gzip_crc.sh still succeeds with darcs 2.0.0 (unknown). With darcs 2.0.2, I get this error:

> $ uname -a
> Darwin Thorkil-Naurs-Computer.local 8.11.0 Darwin Kernel Version 8.11.0: Wed 
Oct 10 18:26:00 PDT 2007; root:xnu-792.24.17~1/RELEASE_PPC Power Macintosh 
powerpc
> $ darcs --version
> 2.0.2 (release)
> $ sh issue844_gzip_crc.sh 
> rm -rf temp1 temp2
> mkdir temp1
> cd temp1
> darcs init
> echo > a
> darcs add a
> darcs record a -a -m "init"
> Recording changes in "a":
> 
> Darcs needs to know what name (conventionally an email address) to use as the
> patch author, e.g. 'Fred Bloggs <fred@bloggs.invalid>'.  If you provide one
> now it will be stored in the file '_darcs/prefs/author' and used as a default
> in the future.  To change your preferred author address, simply delete or edit
> this file.
> 
> What is your email address? thorkil
> Finished recording patch 'init'
> cd ..
> 
> mkdir temp2
> cd temp2
> darcs init
> darcs pull ../temp1 -a
> Finished pulling and applying.
> # darcs optimize --compress
> for f in _darcs/patches/*-*; do
>         gzip -t < "$f"
> done
> 
> gzip: stdin: invalid compressed data--crc error
> $

Best regards
Thorkil

Ok, to sum up: we think this bug was introduced in darcs 2.0.2.

See http://lists.osuosl.org/pipermail/darcs-users/2008-November/016276.html for
our zlib plans.

I'm marking this as need-info until we hear from Duncan about the new zlib binding.

I'm marking this as resolved, now that Ganesh's patches are merged.