From 9cabd155fdf14b9ba8d240b42b2403e1da178a92 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 7 Feb 2023 00:35:51 -0800 Subject: [PATCH 1/3] return error code when benchmark fails such scenario can happen, for example, when trying a decompression-only benchmark on invalid data. Other possibilities include an allocation error in an intermediate step. So far, the benchmark would return immediately, but still return 0. On command line, this would be confusing, as the program appears successful (though it does not display any successful message). Now it returns !0, which can be interpreted as an error by command line. --- programs/zstdcli.c | 13 ++++++++----- tests/playTests.sh | 2 ++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/programs/zstdcli.c b/programs/zstdcli.c index 660e66bb619..93f75e21d9d 100644 --- a/programs/zstdcli.c +++ b/programs/zstdcli.c @@ -1370,7 +1370,7 @@ int main(int argCount, const char* argv[]) benchParams.ldmFlag = ldmFlag; benchParams.ldmMinMatch = (int)g_ldmMinMatch; benchParams.ldmHashLog = (int)g_ldmHashLog; - benchParams.useRowMatchFinder = useRowMatchFinder; + benchParams.useRowMatchFinder = (int)useRowMatchFinder; if (g_ldmBucketSizeLog != LDM_PARAM_DEFAULT) { benchParams.ldmBucketSizeLog = (int)g_ldmBucketSizeLog; } @@ -1391,15 +1391,18 @@ int main(int argCount, const char* argv[]) int c; DISPLAYLEVEL(3, "Benchmarking %s \n", filenames->fileNames[i]); for(c = cLevel; c <= cLevelLast; c++) { - BMK_benchFilesAdvanced(&filenames->fileNames[i], 1, dictFileName, c, &compressionParams, g_displayLevel, &benchParams); + BMK_benchOutcome_t const bo = BMK_benchFilesAdvanced(&filenames->fileNames[i], 1, dictFileName, c, &compressionParams, g_displayLevel, &benchParams); + if (!BMK_isSuccessful_benchOutcome(bo)) return 1; } } } else { for(; cLevel <= cLevelLast; cLevel++) { - BMK_benchFilesAdvanced(filenames->fileNames, (unsigned)filenames->tableSize, dictFileName, cLevel, &compressionParams, g_displayLevel, &benchParams); + BMK_benchOutcome_t const bo = BMK_benchFilesAdvanced(filenames->fileNames, (unsigned)filenames->tableSize, dictFileName, cLevel, &compressionParams, g_displayLevel, &benchParams); + if (!BMK_isSuccessful_benchOutcome(bo)) return 1; } } } else { for(; cLevel <= cLevelLast; cLevel++) { - BMK_syntheticTest(cLevel, compressibility, &compressionParams, g_displayLevel, &benchParams); + BMK_benchOutcome_t const bo = BMK_syntheticTest(cLevel, compressibility, &compressionParams, g_displayLevel, &benchParams); + if (!BMK_isSuccessful_benchOutcome(bo)) return 1; } } #else @@ -1545,7 +1548,7 @@ int main(int argCount, const char* argv[]) if (g_ldmBucketSizeLog != LDM_PARAM_DEFAULT) FIO_setLdmBucketSizeLog(prefs, (int)g_ldmBucketSizeLog); if (g_ldmHashRateLog != LDM_PARAM_DEFAULT) FIO_setLdmHashRateLog(prefs, (int)g_ldmHashRateLog); FIO_setAdaptiveMode(prefs, adapt); - FIO_setUseRowMatchFinder(prefs, useRowMatchFinder); + FIO_setUseRowMatchFinder(prefs, (int)useRowMatchFinder); FIO_setAdaptMin(prefs, adaptMin); FIO_setAdaptMax(prefs, adaptMax); FIO_setRsyncable(prefs, rsyncable); diff --git a/tests/playTests.sh b/tests/playTests.sh index 5d78e9e7d99..5f595f61154 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -1217,6 +1217,8 @@ zstd -rqi0b1e2 tmp1 println "benchmark decompression only" zstd -f tmp1 zstd -b -d -i0 tmp1.zst +println "benchmark can fail - decompression on invalid data" +zstd -b -d -i0 tmp1 && die "invalid .zst data => benchmark should have failed" GZIPMODE=1 zstd --format=gzip -V || GZIPMODE=0 From 58e7067c7d500537bfdccc5ee4707ac0154656d7 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 7 Feb 2023 08:47:39 -0800 Subject: [PATCH 2/3] added more accurate error messages for the decompression-only benchmark mode. --- programs/benchzstd.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/programs/benchzstd.c b/programs/benchzstd.c index 7770dd2a51e..e9deeb6fa7b 100644 --- a/programs/benchzstd.c +++ b/programs/benchzstd.c @@ -327,26 +327,31 @@ BMK_benchMemAdvancedNoAlloc( /* init */ memset(&benchResult, 0, sizeof(benchResult)); if (strlen(displayName)>17) displayName += strlen(displayName) - 17; /* display last 17 characters */ - if (adv->mode == BMK_decodeOnly) { /* benchmark only decompression : source must be already compressed */ + if (adv->mode == BMK_decodeOnly) { + /* benchmark only decompression : source must be already compressed */ const char* srcPtr = (const char*)srcBuffer; U64 totalDSize64 = 0; U32 fileNb; for (fileNb=0; fileNb decodedSize) { /* size_t overflow */ + RETURN_ERROR(32, BMK_benchOutcome_t, "decompressed size is too large for local system"); + } *resultBufferPtr = malloc(decodedSize); if (!(*resultBufferPtr)) { - RETURN_ERROR(33, BMK_benchOutcome_t, "not enough memory"); - } - if (totalDSize64 > decodedSize) { /* size_t overflow */ - free(*resultBufferPtr); - RETURN_ERROR(32, BMK_benchOutcome_t, "original size is too large"); + RETURN_ERROR(33, BMK_benchOutcome_t, "allocation error: not enough memory"); } cSize = srcSize; srcSize = decodedSize; @@ -474,7 +479,7 @@ BMK_benchMemAdvancedNoAlloc( BMK_runOutcome_t const dOutcome = BMK_benchTimedFn(timeStateDecompress, dbp); if(!BMK_isSuccessful_runOutcome(dOutcome)) { - return BMK_benchOutcome_error(); + RETURN_ERROR(30, BMK_benchOutcome_t, "decompression error"); } { BMK_runTime_t const dResult = BMK_extract_runTime(dOutcome); @@ -598,7 +603,7 @@ BMK_benchOutcome_t BMK_benchMemAdvanced(const void* srcBuffer, size_t srcSize, void* resultBuffer = srcSize ? malloc(srcSize) : NULL; - int allocationincomplete = !srcPtrs || !srcSizes || !cPtrs || + int const allocationincomplete = !srcPtrs || !srcSizes || !cPtrs || !cSizes || !cCapacities || !resPtrs || !resSizes || !timeStateCompress || !timeStateDecompress || !cctx || !dctx || From 6740f8f0b8720ded3e8fa35f546115ac29c60e2e Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 7 Feb 2023 10:02:09 -0800 Subject: [PATCH 3/3] add error message for the (rare) compression error scenario --- programs/benchzstd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/benchzstd.c b/programs/benchzstd.c index e9deeb6fa7b..a76db5f375f 100644 --- a/programs/benchzstd.c +++ b/programs/benchzstd.c @@ -451,7 +451,7 @@ BMK_benchMemAdvancedNoAlloc( BMK_runOutcome_t const cOutcome = BMK_benchTimedFn( timeStateCompress, cbp); if (!BMK_isSuccessful_runOutcome(cOutcome)) { - return BMK_benchOutcome_error(); + RETURN_ERROR(30, BMK_benchOutcome_t, "compression error"); } { BMK_runTime_t const cResult = BMK_extract_runTime(cOutcome);