recode 3.7.14: enca tests fail

[oreo639]

This is a continuation of #4, while this was supposedly fixed with shlomif@27bfa44, I am still running into test failures with enca and recode 3.7.14.

I noticed this while trying to upgrade Void Linux's recode from 3.6 to 3.7 (after changing -C recode to -C librecode to in test-tex.sh in enca, typo that results in extra warnings):

enca: Cannot seek in file `test-tex.actual': Bad file descriptor
double free or corruption (!prev)
./test-tex.sh: line 15:  8378 Aborted                 $ENCA $OPTS -x UTF-8 $TESTNAME.actual
FAIL test-tex.sh (exit status: 1)

(The test-recode.sh test succeeds though, not sure why)

It appears most distros don't build enca with librecode, so they don't run into this issue.

Using GDB, the error occurs here: https://github.com/nijel/enca/blob/6fc4a2a3e9ec16b892e325f6f53b2501fbcacb17/src/convert_recode.c#L127

For context here is the backtrace:

(gdb) run -L cs -C librecode -x UTF-8 test-tex.actual
Starting program: /home/oreo639/git/IPR/vp2/masterdir-x86_64/builddir/enca-1.19/src/.libs/enca -L cs -C librecode -x UTF-8 test-tex.actual
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib64/libthread_db.so.1".
[Detaching after fork from child process 6495]
free(): invalid pointer

Program received signal SIGABRT, Aborted.
__pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, 
    no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
warning: 44	./nptl/pthread_kill.c: No such file or directory
(gdb) bt
#0  __pthread_kill_implementation (threadid=<optimized out>, 
    signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
#1  0x00007ffff7cc525f in __pthread_kill_internal (signo=6, 
    threadid=<optimized out>) at ./nptl/pthread_kill.c:89
#2  0x00007ffff7c75fe2 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/posix/raise.c:26
#3  0x00007ffff7c5eefc in __GI_abort () at ./stdlib/abort.c:73
#4  0x00007ffff7c5fd4c in __libc_message_impl (
    fmt=fmt@entry=0x7ffff7dd5328 "%s\n") at ../sysdeps/posix/libc_fatal.c:134
#5  0x00007ffff7ccea77 in malloc_printerr (
    str=str@entry=0x7ffff7dd30f9 "free(): invalid pointer")
    at ./malloc/malloc.c:5829
#6  0x00007ffff7cd391c in _int_free_check (av=0x7ffff7e11ac0 <main_arena>, 
    size=<optimized out>, p=0x55555556a3a3) at ./malloc/malloc.c:4560
#7  _int_free (have_lock=0, p=0x55555556a3a3, av=0x7ffff7e11ac0 <main_arena>)
    at ./malloc/malloc.c:4692
#8  __GI___libc_free (mem=0x55555556a3b3) at ./malloc/malloc.c:3476
#9  0x00007ffff7cbd26e in _IO_free_backup_buf (ptr=<optimized out>, 
    fp=0x55555556a330) at ./libio/libioP.h:1050
#10 __GI__IO_free_backup_area (fp=fp@entry=0x55555556a330)
    at ./libio/genops.c:216
#11 0x00007ffff7cbbaa0 in _IO_new_file_seekoff (fp=0x55555556a330, offset=0, 
    dir=0, mode=<optimized out>) at ./libio/fileops.c:977
#12 0x00007ffff7cb7c05 in __GI_fseek (fp=0x55555556a330, 
--Type <RET> for more, q to quit, c to continue without paging--c
    offset=offset@entry=0, whence=whence@entry=0) at ./libio/fseek.c:36
#13 0x000055555555c8c7 in file_seek (file=file@entry=0x55555556a5b0, 
    offset=offset@entry=0, whence=whence@entry=0) at ./src/filebuf.c:358
#14 0x000055555555ac6f in convert_recode (file=0x55555556a5b0, from_enc=...)
    at ./src/convert_recode.c:127
#15 0x000055555555a459 in convert (file=file@entry=0x55555556a5b0, 
    from_enc=...) at ./src/convert.c:134
#16 0x000055555555b8b1 in process_file (an=an@entry=0x55555556a620, 
    fname=<optimized out>) at ./src/enca.c:150
#17 0x0000555555559660 in main (argc=<optimized out>, argv=<optimized out>)
    at ./src/enca.c:78
(gdb) 

[rrthomas]

Thanks for this. What would be great is to run the tests with Valgrind or similar and see where the file descriptor is actually closed (or otherwise freed).

[oreo639]

Done: https://gist.github.com/oreo639/3a927a0a7e313ba6c70fd9955834e293

Seems to be happening here: https://github.com/rrthomas/recode/blob/v3.7.14/src/task.c#L359
So, yeah, this is a different issue from the previous one (in the sense that code was added later).

[rrthomas]

Many thanks for the instant reply! I will look into this. Looks like another instance of the same problem, i.e. recode closing a file descriptor that it doesn't own.

[rrthomas]

Many apologies, this bug was I believe fixed in commit 80516f6 which was made post-3.7.14 at a time when I meant to make a major release shortly thereafter, but I've still not gotten around to.

If you can try building and testing enca with current git HEAD of recode to confirm, that would be great; in any case, I'll make a new release shortly.

[oreo639]

Ah, yeah. I assumed that was already in 3.7.14 (since the reporter was using 3.7.13).

That patch does seem to resolve the test failure.
Thanks for looking into it.

Edit: still seems to be failing on musl, give me a moment.
Edit 2: the musl issue seems to be a different problem (not double freeing FILE*, but recode_perform_task() is returning an error which doesn't happen with 3.6.x)

[rrthomas]

Great, so sounds like this issue can be closed, but another is required for the musl failure?

[oreo639]

Yeah.

[rrthomas]

Great, that's one down. With luck we can get the other fixed and I can get a new minor release out.

[oreo639]

Regarding the musl issue. After checking the version of librecode 3.6 we are shipping, it includes a copy of libiconv: https://git.savannah.gnu.org/gitweb/?p=libiconv.git;a=tree;f=lib;h=5ba3b8a377cf6d33af783892a90faee9ac9df178;hb=HEAD

I see there is a branch for that here: https://github.com/rrthomas/recode/tree/static-iconv

Should I still report that as a bug, since the issue is simply that musl provides a very minimal iconv implementation?

[rrthomas]

Aha! Yes indeed, that's not a Recode issue. The branch, you may have noticed, is quite old; it is proving tricky to integrate libiconv, but I have an offer of help from its author, so I must get back to that. In the mean time, you can of course install libiconv normally on a musl system and use that to build Recode 3.7.x.