libticalcs test-suite fails

[and1bm]

Hi, I am in the process packaging libtiXXX latest git HEAD for Debian. Unfortunately, the testsuite for libticalcs fails:

[…]
Making check in tests
make[2]: Entering directory '/<<PKGBUILDDIR>>/tests'
make  check-TESTS
make[3]: Entering directory '/<<PKGBUILDDIR>>/tests'
make[4]: Entering directory '/<<PKGBUILDDIR>>/tests'
../test-driver: line 112: 414584 Segmentation fault      "$@" >> "$log_file" 2>&1
FAIL: torture_ticalcs
==============================================
   libticalcs2 1.1.10: tests/test-suite.log
==============================================

# TOTAL: 1
# PASS:  0
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: torture_ticalcs
=====================
(process:414584): ticalcs-CRITICAL **: 10:15:24.986: ticalcs_error_get(NULL)
[…]

Any help is appreciated! Thanks and best regards, Andi

[debrouxl]

Thanks for the report :)

It's interesting that I didn't see it less than 10 days ago, the last time I built the libti*/gfm/tilp stack, using the usual recompile_tilp.sh maintainer script (part of the tilp_and_gfm repository), which passes to the toolchain a set of build flags similar to the ones used by Debian hardened package builds (and recently, -D_FORTIFY_SOURCE=3). I'll try to reproduce on my side; still, please post the libticalcs/trunk/tests/torture_ticalcs.log file you should have obtained. TIA :)

[and1bm]

Hi Lionel, thanks for your quick answer! Here is libticalcs/trunk/tests/torture_ticalcs.log: torture_ticalcs.log

[debrouxl]

I'm at a loss about what's happening :)

Given the following code:

torture_ticalcs.c:
static void torture_cmd68k(void)
{
    ...
    PRINTF(ti92_recv_RTS, INT, NULL, (void *)0x12345678, (void *)0x12345678, (void *)0x12345678);
    PRINTF(ti92_recv_RTS, INT, (void *)0x12345678, NULL, (void *)0x12345678, (void *)0x12345678);
    PRINTF(ti92_recv_RTS, INT, (void *)0x12345678, (void *)0x12345678, NULL, (void *)0x12345678);
    PRINTF(ti92_recv_RTS, INT, (void *)0x12345678, (void *)0x12345678, (void *)0x12345678, NULL);
}

static void torture_romdump(void)
{
// romdump.c
    PRINTF(rd_send_dumper, INT, NULL, (void *)0x12345678, 0, (void *)0x12345678);
    PRINTF(rd_send_dumper, INT, (void *)0x12345678, NULL, 0, (void *)0x12345678);
    PRINTF(rd_send_dumper, INT, (void *)0x12345678, (void *)0x12345678, 0, NULL);
    PRINTF(rd_send_dumper, INT, (void *)0x12345678, (void *)0x12345678, 0, (void *)0x12345678);
    ...
}

...

static void dissect_functions_unit_test_1(void)
{
    assert(ERR_INVALID_PACKET == dbus_dissect(CALC_NONE, stderr, (void *)0x12345678, 1));
    ...
}

...

int main(int argc, char **argv)
{
    ...
    torture_cmd68k();
    torture_romdump();
    dissect_functions_unit_test_1();
    ...
}

the first call to rd_send_dumper would be the one triggering a segfault on your side... but that really should not occur:

romdump.cc:
TIEXPORT3 int TICALL rd_send_dumper(CalcHandle *handle, const char *prgname, uint16_t size, uint8_t *data)
{
    char *templatename, *tempfname;
    int fd, ret;

    VALIDATE_HANDLE(handle);
    VALIDATE_NONNULL(prgname);
    VALIDATE_NONNULL(data);

    if (size < 64)
    {
        // File too short.
        return ERR_INVALID_PARAMETER;
    }

    ...
}

internal.h:
#define VALIDATE_NONNULL(ptr) \
    do \
    { \
        if (ptr == NULL) \
        { \
            ticalcs_critical("%s: " #ptr " is NULL", __FUNCTION__); \
            return ERR_INVALID_PARAMETER; \
        } \
    } while(0);
#define VALIDATE_HANDLE(handle) \
    do \
    { \
        if (!ticalcs_validate_handle(handle)) \
        { \
            ticalcs_critical("%s: " #handle " is invalid", __FUNCTION__); \
            return ERR_INVALID_HANDLE; \
        } \
    } while(0);

static inline int ticalcs_validate_handle(CalcHandle * handle)
{
    return handle != NULL;
}

dbus_pkt.cc:
TIEXPORT3 int TICALL dbus_dissect(CalcModel model, FILE * f, const uint8_t * data, uint32_t len)
{
    ...
    VALIDATE_NONNULL(f);
    VALIDATE_NONNULL(data);

    if (len < 2 || len == 3 || len > 65536U + 6)
    {
        ticalcs_critical("Length %lu (%lX) is too small or too large for a valid DBUS packet", (unsigned long)len, (unsigned long)len);
        return ERR_INVALID_PACKET;
    }

    ...
}

None of the VALIDATE_* macros dereferences its argument, and with a given size of 0, rd_send_dumper exits early. On my side, torture_ticalcs.log contains the expected content:

(process:4086384): ticalcs-CRITICAL **: 17:58:19.918: ti68k_recv_RTS: varname is NULL
745     283

(process:4086384): ticalcs-CRITICAL **: 17:58:19.918: rd_send_dumper: handle is invalid
751     282

(process:4086384): ticalcs-CRITICAL **: 17:58:19.918: rd_send_dumper: prgname is NULL
752     283

(process:4086384): ticalcs-CRITICAL **: 17:58:19.918: rd_send_dumper: data is NULL
753     283
754     283
...

The function wasn't exported until f2d61881844c28a14eef27df5e1798cdcc1ec873 , which is two commits behind current master HEAD, so you can't be running a new torture_ticalcs against an old libticalcs binary: it wouldn't compile or load in the first place. An older torture_ticalcs not containing the romdump function torture tests would crash in dbus_dissect if that function were buggy, but per the above, it isn't either, and it hasn't changed for years.

[and1bm]

I'll investigate, perhaps it's related to patches/modifications done in the Debian package. Some ROM dumpers are removed as there is no source available, perhaps this doesn't work anymore. Thank you for your help so far!

[debrouxl]

FTR, there will soon be a single ROM dumper which can't be recompiled using packages from the Debian repository: the TI-eZ80 ROM dumper. I converted the TI-Z80 dumpers to pasmo, and rewrote the TI-68k dumper in pure assembly so that it can be assembled by m68k-elf gas. The patches have lived on the experimental2 branch for a long time by now.

[and1bm]

Hey, I dropped part of the Debian-patch and all tests work again as expected! Sorry for the noise, my fault.

Looking forward to the new dumper!