From 4318cc22ee10b5f9702744d3097aea86f449afaf Mon Sep 17 00:00:00 2001 From: "Markus F.X.J. Oberhumer" Date: Sun, 11 Sep 2022 15:58:37 +0200 Subject: [PATCH] src: cleanup option handling, add testcases; fixes #587 --- src/Makefile | 2 +- src/conf.h | 4 +- src/main.cpp | 108 ++++++++++------------------------------ src/options.cpp | 129 ++++++++++++++++++++++++++++++++++++++++++++++++ src/options.h | 12 ++--- src/p_exe.cpp | 7 +-- src/packer.cpp | 2 +- src/work.cpp | 14 +++--- 8 files changed, 175 insertions(+), 103 deletions(-) create mode 100644 src/options.cpp diff --git a/src/Makefile b/src/Makefile index 55fb3b6a..b36a99ea 100644 --- a/src/Makefile +++ b/src/Makefile @@ -209,7 +209,7 @@ ifeq ($(shell uname),Linux) CLANG_FORMAT_FILES += bele.h bele_policy.h CLANG_FORMAT_FILES += dt_check.cpp dt_impl.cpp except.cpp except.h CLANG_FORMAT_FILES += linker.cpp linker.h packhead.cpp packmast.cpp packmast.h -CLANG_FORMAT_FILES += main.cpp options.h packer.cpp packer.h +CLANG_FORMAT_FILES += main.cpp options.cpp options.h packer.cpp packer.h CLANG_FORMAT_FILES += p_tos.cpp p_tos.h CLANG_FORMAT_FILES += s_djgpp2.cpp s_object.cpp s_vcsa.cpp s_win32.cpp screen.h CLANG_FORMAT_FILES += snprintf.cpp diff --git a/src/conf.h b/src/conf.h index 32ec3d60..44ac0a2e 100644 --- a/src/conf.h +++ b/src/conf.h @@ -738,7 +738,9 @@ bool upx_doctest_check(void); // main.cpp extern const char *progname; -bool set_exit_code(int ec); +bool main_set_exit_code(int ec); +int main_get_options(int argc, char **argv); +void main_get_envoptions(); int upx_main(int argc, char *argv[]); // msg.cpp diff --git a/src/main.cpp b/src/main.cpp index f1278340..ae2da383 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -35,48 +35,7 @@ // options **************************************************************************/ -void options_t::reset() { - options_t *o = this; - mem_clear(o, sizeof(*o)); - o->crp.reset(); - - o->cmd = CMD_NONE; - o->method = M_NONE; - o->level = -1; - o->filter = FT_NONE; - - o->backup = -1; - o->overlay = -1; - o->preserve_mode = true; - o->preserve_ownership = true; - o->preserve_timestamp = true; - - o->console = CON_FILE; -#if (ACC_OS_DOS32) && defined(__DJGPP__) - o->console = CON_INIT; -#elif (USE_SCREEN_WIN32) - o->console = CON_INIT; -#elif 1 && defined(__linux__) - o->console = CON_INIT; -#endif - o->verbose = 2; - - o->win32_pe.compress_exports = 1; - o->win32_pe.compress_icons = 2; - o->win32_pe.compress_resources = -1; - for (unsigned i = 0; i < TABLESIZE(o->win32_pe.compress_rt); i++) - o->win32_pe.compress_rt[i] = -1; - o->win32_pe.compress_rt[24] = false; // 24 == RT_MANIFEST - o->win32_pe.strip_relocs = -1; - o->win32_pe.keep_resource = ""; -} - -static options_t global_options; -options_t *opt = &global_options; - -static int done_output_name = 0; - -const char *argv0 = ""; +static const char *argv0 = ""; const char *progname = ""; static acc_getopt_t mfx_getopt; @@ -138,14 +97,18 @@ static bool set_eec(int ec, int *eec) { return 0; } -bool set_exit_code(int ec) { return set_eec(ec, &exit_code); } +bool main_set_exit_code(int ec) { return set_eec(ec, &exit_code); } static void e_exit(int ec) { - (void) set_exit_code(ec); + if (opt->debug.getopt_throw_instead_of_exit) + throw ec; + (void) main_set_exit_code(ec); do_exit(); } static void e_usage(void) { + if (opt->debug.getopt_throw_instead_of_exit) + throw EXIT_USAGE; show_usage(); e_exit(EXIT_USAGE); } @@ -287,7 +250,7 @@ static bool set_method(int m, int l) { static void set_output_name(const char *n, bool allow_m) { #if 1 - if (done_output_name > 0) { + if (opt->output_name) { fprintf(stderr, "%s: option '-o' more than once given\n", argv0); e_usage(); } @@ -301,7 +264,6 @@ static void set_output_name(const char *n, bool allow_m) { e_usage(); } opt->output_name = n; - done_output_name++; } /************************************************************************* @@ -482,13 +444,13 @@ static int do_option(int optc, const char *arg) { break; case 721: opt->method_lzma_seen = true; - opt->all_methods_use_lzma = true; + opt->all_methods_use_lzma = 1; if (!set_method(M_LZMA, -1)) e_method(M_LZMA, opt->level); break; case 722: opt->method_lzma_seen = false; - opt->all_methods_use_lzma = false; + opt->all_methods_use_lzma = -1; // explicitly disabled if (M_IS_LZMA(opt->method)) opt->method = -1; break; @@ -518,7 +480,8 @@ static int do_option(int optc, const char *arg) { /* fallthrough */ case 901: // --brute opt->all_methods = true; - opt->all_methods_use_lzma = true; + if (opt->all_methods_use_lzma != -1) + opt->all_methods_use_lzma = 1; opt->method = -1; opt->all_filters = true; opt->filter = -1; @@ -549,17 +512,6 @@ static int do_option(int optc, const char *arg) { opt->debug.disable_random_id = true; break; - // mp (meta) - case 501: - getoptvar(&opt->mp_compress_task, 1, 999999, arg); - break; - case 502: - opt->mp_query_format = true; - break; - case 503: - opt->mp_query_num_tasks = true; - break; - // misc case 512: opt->console = CON_FILE; @@ -606,7 +558,8 @@ static int do_option(int optc, const char *arg) { break; case 524: // --all-methods opt->all_methods = true; - opt->all_methods_use_lzma = true; + if (opt->all_methods_use_lzma != -1) + opt->all_methods_use_lzma = 1; opt->method = -1; break; case 525: // --exact @@ -820,7 +773,7 @@ static int do_option(int optc, const char *arg) { return 0; } -static int get_options(int argc, char **argv) { +int main_get_options(int argc, char **argv) { constexpr int *N = nullptr; static const struct mfx_option longopts[] = { @@ -852,9 +805,9 @@ static int get_options(int argc, char **argv) { {"quiet", 0, N, 'q'}, // quiet mode {"silent", 0, N, 'q'}, // quiet mode #if 0 - // FIXME: to_stdout doesn't work because of console code mess - {"stdout", 0x10, N, 517}, // write output on standard output - {"to-stdout", 0x10, N, 517}, // write output on standard output + // FIXME: to_stdout doesn't work because of console code mess + {"stdout", 0x10, N, 517}, // write output on standard output + {"to-stdout", 0x10, N, 517}, // write output on standard output #endif {"verbose", 0, N, 'v'}, // verbose mode @@ -970,11 +923,6 @@ static int get_options(int argc, char **argv) { {"8mib-ram", 0x10, N, 673}, {"8mb-ram", 0x10, N, 673}, - // mp (meta) options - {"mp-compress-task", 0x31, N, 501}, - {"mp-query-format", 0x10, N, 502}, - {"mp-query-num-tasks", 0x10, N, 503}, - {nullptr, 0, nullptr, 0} }; @@ -994,8 +942,8 @@ static int get_options(int argc, char **argv) { return mfx_optind; } +void main_get_envoptions() { #if defined(OPTIONS_VAR) -static void get_envoptions(int argc, char **argv) { constexpr int *N = nullptr; /* only some options are allowed in the environment variable */ @@ -1068,7 +1016,7 @@ static void get_envoptions(int argc, char **argv) { const char *var; int i, optc, longind; int targc; - char **targv = nullptr; + const char **targv = nullptr; static const char sep[] = " \t"; char shortopts[256]; @@ -1095,14 +1043,14 @@ static void get_envoptions(int argc, char **argv) { /* alloc temp argv */ if (targc > 1) - targv = (char **) calloc(targc + 1, sizeof(char *)); + targv = (const char **) calloc(targc + 1, sizeof(char *)); if (targv == nullptr) { free(env); return; } /* fill temp argv */ - targv[0] = argv[0]; + targv[0] = argv0; for (p = env, targc = 1;;) { while (*p && strchr(sep, *p)) p++; @@ -1124,7 +1072,7 @@ static void get_envoptions(int argc, char **argv) { /* handle options */ prepare_shortopts(shortopts, "123456789", longopts); - acc_getopt_init(&mfx_getopt, 1, targc, targv); + acc_getopt_init(&mfx_getopt, 1, targc, const_cast(targv)); mfx_getopt.progname = progname; mfx_getopt.opterr = handle_opterr; while ((optc = acc_getopt(&mfx_getopt, shortopts, longopts, &longind)) >= 0) { @@ -1138,9 +1086,8 @@ static void get_envoptions(int argc, char **argv) { /* clean up */ free(targv); free(env); - UNUSED(argc); -} #endif /* defined(OPTIONS_VAR) */ +} static void first_options(int argc, char **argv) { int i; @@ -1181,7 +1128,6 @@ int upx_main(int argc, char *argv[]) { } // Allow serial re-use of upx_main() as a subroutine - done_output_name = 0; opt->reset(); #if (ACC_OS_CYGWIN || ACC_OS_DOS16 || ACC_OS_DOS32 || ACC_OS_EMX || ACC_OS_TOS || ACC_OS_WIN16 || \ @@ -1219,11 +1165,9 @@ int upx_main(int argc, char *argv[]) { /* get options */ first_options(argc, argv); -#if defined(OPTIONS_VAR) if (!opt->no_env) - get_envoptions(argc, argv); -#endif - i = get_options(argc, argv); + main_get_envoptions(); + i = main_get_options(argc, argv); assert(i <= argc); set_term(nullptr); diff --git a/src/options.cpp b/src/options.cpp new file mode 100644 index 00000000..f4bf8be7 --- /dev/null +++ b/src/options.cpp @@ -0,0 +1,129 @@ +/* options.cpp -- + + This file is part of the UPX executable compressor. + + Copyright (C) 1996-2022 Markus Franz Xaver Johannes Oberhumer + Copyright (C) 1996-2022 Laszlo Molnar + All Rights Reserved. + + UPX and the UCL library are free software; you can redistribute them + and/or modify them under the terms of the GNU General Public License as + published by the Free Software Foundation; either version 2 of + the License, or (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; see the file COPYING. + If not, write to the Free Software Foundation, Inc., + 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + + Markus F.X.J. Oberhumer Laszlo Molnar + + */ + +#include "conf.h" + +/************************************************************************* +// options +**************************************************************************/ + +void options_t::reset() { + options_t *o = this; + mem_clear(o, sizeof(*o)); + o->crp.reset(); + + o->cmd = CMD_NONE; + o->method = M_NONE; + o->level = -1; + o->filter = FT_NONE; + + o->backup = -1; + o->overlay = -1; + o->preserve_mode = true; + o->preserve_ownership = true; + o->preserve_timestamp = true; + + o->console = CON_FILE; +#if (ACC_OS_DOS32) && defined(__DJGPP__) + o->console = CON_INIT; +#elif (USE_SCREEN_WIN32) + o->console = CON_INIT; +#elif 1 && defined(__linux__) + o->console = CON_INIT; +#endif + o->verbose = 2; + + o->win32_pe.compress_exports = 1; + o->win32_pe.compress_icons = 2; + o->win32_pe.compress_resources = -1; + for (unsigned i = 0; i < TABLESIZE(o->win32_pe.compress_rt); i++) + o->win32_pe.compress_rt[i] = -1; + o->win32_pe.compress_rt[24] = false; // 24 == RT_MANIFEST + o->win32_pe.strip_relocs = -1; + o->win32_pe.keep_resource = ""; +} + +static options_t global_options; +options_t *opt = &global_options; + +/************************************************************************* +// +**************************************************************************/ + +template +static void test_options(const char *(&a)[N]) { + (void) main_get_options((int) (N - 1), ACC_UNCONST_CAST(char **, a)); +} + +TEST_CASE("getopt") { + options_t *saved_opt = opt; + options_t local_options; + opt = &local_options; + opt->reset(); + opt->debug.getopt_throw_instead_of_exit = true; + const char a0[] = ""; + + SUBCASE("issue 587a") { + const char *a[] = {a0, "--brute", "--lzma", nullptr}; + CHECK(!opt->all_methods); + test_options(a); + CHECK(opt->all_methods); + CHECK(opt->all_methods_use_lzma == 1); + } + SUBCASE("issue 587b") { + const char *a[] = {a0, "--lzma", "--brute", nullptr}; + CHECK(!opt->all_methods); + test_options(a); + CHECK(opt->all_methods); + CHECK(opt->all_methods_use_lzma == 1); + } + SUBCASE("issue 587c") { + const char *a[] = {a0, "--brute", "--no-lzma", nullptr}; + CHECK(!opt->all_methods); + test_options(a); + CHECK(opt->all_methods); + CHECK(opt->all_methods_use_lzma == -1); + } + SUBCASE("issue 587d") { + const char *a[] = {a0, "--no-lzma", "--brute", nullptr}; + CHECK(!opt->all_methods); + test_options(a); + CHECK(opt->all_methods); + CHECK(opt->all_methods_use_lzma == -1); + } + SUBCASE("issue 587e") { + const char *a[] = {a0, "--no-lzma", "--all-methods", nullptr}; + CHECK(!opt->all_methods); + test_options(a); + CHECK(opt->all_methods); + CHECK(opt->all_methods_use_lzma == -1); + } + + opt = saved_opt; +} + +/* vim:set ts=4 sw=4 et: */ diff --git a/src/options.h b/src/options.h index 450e273a..d4021502 100644 --- a/src/options.h +++ b/src/options.h @@ -48,11 +48,6 @@ enum { struct options_t { int cmd; - // mp (meta) options - int mp_compress_task; - bool mp_query_format; - bool mp_query_num_tasks; - // compression options int method; bool method_lzma_seen; @@ -63,7 +58,7 @@ struct options_t { int filter; // preferred filter from Packer::getFilters() bool ultra_brute; bool all_methods; // try all available compression methods ? - bool all_methods_use_lzma; + int all_methods_use_lzma; bool all_filters; // try all available filters ? bool no_filter; // force no filter bool prefer_ucl; // prefer UCL @@ -90,8 +85,9 @@ struct options_t { int debug_level; bool disable_random_id; // for Packer::getRandomId() const char *dump_stub_loader; - char fake_stub_version[4 + 1]; // for internal debugging - char fake_stub_year[4 + 1]; // for internal debugging + char fake_stub_version[4 + 1]; // for internal debugging + char fake_stub_year[4 + 1]; // for internal debugging + bool getopt_throw_instead_of_exit; // for doctest } debug; // overlay handling diff --git a/src/p_exe.cpp b/src/p_exe.cpp index 8abe31aa..fe45707c 100644 --- a/src/p_exe.cpp +++ b/src/p_exe.cpp @@ -63,9 +63,10 @@ const int *PackExe::getCompressionMethods(int method, int level) const { bool small = ih_imagesize <= 256*1024; // disable lzma for "--brute" unless explicitly given "--lzma" - // WARNING: this side effect persists for later files! - if (opt->all_methods_use_lzma && !opt->method_lzma_seen) - opt->all_methods_use_lzma = false; + // WARNING: this side effect may persists for later files; + // but note that class PackMaster creates per-file local options + if (opt->all_methods_use_lzma == 1 && !opt->method_lzma_seen) + opt->all_methods_use_lzma = 0; return Packer::getDefaultCompressionMethods_8(method, level, small); } diff --git a/src/packer.cpp b/src/packer.cpp index b552d999..4d085ac2 100644 --- a/src/packer.cpp +++ b/src/packer.cpp @@ -1165,7 +1165,7 @@ int Packer::prepareMethods(int *methods, int ph_method, const int *all_methods) break; if (method == M_SKIP || method == M_ULTRA_BRUTE) continue; - if (opt->all_methods && !opt->all_methods_use_lzma && M_IS_LZMA(method)) + if (opt->all_methods && opt->all_methods_use_lzma != 1 && M_IS_LZMA(method)) continue; // use this method assert(Packer::isValidCompressionMethod(method)); diff --git a/src/work.cpp b/src/work.cpp index c36120c0..5effbfe4 100644 --- a/src/work.cpp +++ b/src/work.cpp @@ -271,40 +271,40 @@ int do_files(int i, int argc, char *argv[]) { unlink_ofile(oname); if (opt->verbose >= 1 || (opt->verbose >= 0 && !e.isWarning())) printErr(iname, &e); - set_exit_code(e.isWarning() ? EXIT_WARN : EXIT_ERROR); + main_set_exit_code(e.isWarning() ? EXIT_WARN : EXIT_ERROR); // continue processing more files } catch (const Error &e) { unlink_ofile(oname); printErr(iname, &e); - set_exit_code(EXIT_ERROR); + main_set_exit_code(EXIT_ERROR); return -1; } catch (std::bad_alloc *e) { unlink_ofile(oname); printErr(iname, "out of memory"); UNUSED(e); // delete e; - set_exit_code(EXIT_ERROR); + main_set_exit_code(EXIT_ERROR); return -1; } catch (const std::bad_alloc &) { unlink_ofile(oname); printErr(iname, "out of memory"); - set_exit_code(EXIT_ERROR); + main_set_exit_code(EXIT_ERROR); return -1; } catch (std::exception *e) { unlink_ofile(oname); printUnhandledException(iname, e); // delete e; - set_exit_code(EXIT_ERROR); + main_set_exit_code(EXIT_ERROR); return -1; } catch (const std::exception &e) { unlink_ofile(oname); printUnhandledException(iname, &e); - set_exit_code(EXIT_ERROR); + main_set_exit_code(EXIT_ERROR); return -1; } catch (...) { unlink_ofile(oname); printUnhandledException(iname, nullptr); - set_exit_code(EXIT_ERROR); + main_set_exit_code(EXIT_ERROR); return -1; } }