diff options
28 files changed, 174 insertions, 3 deletions
diff --git a/changes/ticket31634 b/changes/ticket31634 new file mode 100644 index 0000000000..2777595036 --- /dev/null +++ b/changes/ticket31634 @@ -0,0 +1,4 @@ + o Minor features (testing, architeture): + - Our test scripts now double-check that subsystem initialization order + is consistent with the inter-module dependencies established by our + .may_include files. Implements ticket 31634. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 7126cb8a58..f2acf15f12 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -186,6 +186,13 @@ The following options in this section are only recognized on the ISO-8601 format. For example, the output sent to stdout will be of the form: "signing-cert-expiry: 2017-07-25 08:30:15 UTC" +[[opt-dbg]] **--dbg-**...:: + Tor may support other options beginning with the string "dbg". These + are intended for use by developers to debug and test Tor. They are + not supported or guaranteed to be stable, and you should probably + not use them. + + [[conf-format]] == THE CONFIGURATION FILE FORMAT diff --git a/scripts/maint/practracker/includes.py b/scripts/maint/practracker/includes.py index e9b02c35b0..a5ee728824 100755 --- a/scripts/maint/practracker/includes.py +++ b/scripts/maint/practracker/includes.py @@ -59,6 +59,8 @@ ALLOWED_PATTERNS = [ re.compile(r'^micro-revision.i$'), ] +TOPDIR = "src" + def pattern_is_normal(s): for p in ALLOWED_PATTERNS: if p.match(s): @@ -136,6 +138,29 @@ class Rules(object): return allowed + +def normalize_srcdir(fname): + """given the name of a source directory or file, return its name + relative to `src` in a unix-like format. + """ + orig = fname + dirname, dirfile = os.path.split(fname) + if re.match(r'.*\.[ch]$', dirfile): + fname = dirname + + # Now we have a directory. + dirname, result = os.path.split(fname) + for _ in range(100): + # prevent excess looping in case I missed a tricky case + dirname, dirpart = os.path.split(dirname) + if dirpart == 'src' or dirname == "": + #print(orig,"=>",result) + return result + result = "{}/{}".format(dirpart,result) + + print("No progress!") + assert False + include_rules_cache = {} def load_include_rules(fname): @@ -173,6 +198,27 @@ def remove_self_edges(graph): for k in list(graph): graph[k] = [ d for d in graph[k] if d != k ] +def closure(graph): + """Takes a directed graph in as an adjacency mapping (a mapping from + node to a list of the nodes to which it connects), and completes + its closure. + """ + graph = graph.copy() + changed = False + for k in graph.keys(): + graph[k] = set(graph[k]) + while True: + for k in graph.keys(): + sz = len(graph[k]) + for v in list(graph[k]): + graph[k].update(graph.get(v, [])) + if sz != len(graph[k]): + changed = True + + if not changed: + return graph + changed = False + def toposort(graph, limit=100): """Takes a directed graph in as an adjacency mapping (a mapping from node to a list of the nodes to which it connects). Tries to @@ -233,8 +279,38 @@ def walk_c_files(topdir="src"): for err in consider_include_rules(fullpath, f): yield err +def open_or_stdin(fname): + if fname == '-': + return sys.stdin + else: + return open(fname) + +def check_subsys_file(fname, uses_dirs): + if not uses_dirs: + # We're doing a distcheck build, or for some other reason there are + # no .may_include files. + print("SKIPPING") + return False + + uses_dirs = { normalize_srcdir(k) : { normalize_srcdir(d) for d in v } + for (k,v) in uses_dirs.items() } + uses_closure = closure(uses_dirs) + ok = True + previous_subsystems = [] + + with open_or_stdin(fname) as f: + for line in f: + _, name, fname = line.split() + fname = normalize_srcdir(fname) + for prev in previous_subsystems: + if fname in uses_closure[prev]: + print("INVERSION: {} uses {}".format(prev,fname)) + ok = False + previous_subsystems.append(fname) + return not ok + def run_check_includes(topdir, list_unused=False, log_sorted_levels=False, - list_advisories=False): + list_advisories=False, check_subsystem_order=None): trouble = False for err in walk_c_files(topdir): @@ -259,6 +335,11 @@ def run_check_includes(topdir, list_unused=False, log_sorted_levels=False, uses_dirs[rules.incpath] = rules.getAllowedDirectories() remove_self_edges(uses_dirs) + + if check_subsystem_order: + if check_subsys_file(check_subsystem_order, uses_dirs): + sys.exit(1) + all_levels = toposort(uses_dirs) if log_sorted_levels: @@ -282,14 +363,19 @@ def main(argv): help="List unused lines in .may_include files.") parser.add_argument("--list-advisories", action="store_true", help="List advisories as well as forbidden includes") + parser.add_argument("--check-subsystem-order", action="store", + help="Check a list of subsystems for ordering") parser.add_argument("topdir", default="src", nargs="?", help="Top-level directory for the tor source") args = parser.parse_args(argv[1:]) + global TOPDIR + TOPDIR = args.topdir run_check_includes(topdir=args.topdir, log_sorted_levels=args.toposort, list_unused=args.list_unused, - list_advisories=args.list_advisories) + list_advisories=args.list_advisories, + check_subsystem_order=args.check_subsystem_order) if __name__ == '__main__': main(sys.argv) diff --git a/scripts/maint/run_check_subsystem_order.sh b/scripts/maint/run_check_subsystem_order.sh new file mode 100755 index 0000000000..4ec73bfd56 --- /dev/null +++ b/scripts/maint/run_check_subsystem_order.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash + +set -e + +TOR="${abs_top_builddir:-.}/src/app/tor" + +INCLUDES_PY="${abs_top_srcdir:-.}/scripts/maint/practracker/includes.py" + +if ! test -x "${INCLUDES_PY}" ; then + echo "skip" + exit 77 +fi + +"${TOR}" --dbg-dump-subsystem-list | \ + "${INCLUDES_PY}" --check-subsystem-order - "${abs_top_srcdir}/src" + +echo ok diff --git a/src/app/config/config.c b/src/app/config/config.c index 4becae4756..0ae650eb08 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -2498,6 +2498,9 @@ static const struct { .command=CMD_IMMEDIATE }, { .name="--nt-service" }, { .name="-nt-service" }, + { .name="--dbg-dump-subsystem-list", + .command=CMD_IMMEDIATE, + .quiet=QUIET_HUSH }, { .name=NULL }, }; @@ -4604,6 +4607,10 @@ options_init_from_torrc(int argc, char **argv) list_deprecated_options(); return 1; } + if (config_line_find(cmdline_only_options, "--dbg-dump-subsystem-list")) { + subsystems_dump_list(); + return 1; + } if (config_line_find(cmdline_only_options, "--version")) { printf("Tor version %s.\n",get_version()); diff --git a/src/app/main/subsysmgr.c b/src/app/main/subsysmgr.c index 5807cbbaa4..de601d28cd 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -294,6 +294,20 @@ subsystems_thread_cleanup(void) } /** + * Dump a human- and machine-readable list of all the subsystems to stdout, + * in their initialization order, prefixed with their level. + **/ +void +subsystems_dump_list(void) +{ + for (unsigned i = 0; i < n_tor_subsystems - 1; ++i) { + const subsys_fns_t *sys = tor_subsystems[i]; + printf("% 4d\t%16s\t%s\n", sys->level, sys->name, + sys->location?sys->location:""); + } +} + +/** * Register all subsystem-declared options formats in <b>mgr</b>. * * Return 0 on success, -1 on failure. diff --git a/src/app/main/subsysmgr.h b/src/app/main/subsysmgr.h index 35635a756e..ae0b3df469 100644 --- a/src/app/main/subsysmgr.h +++ b/src/app/main/subsysmgr.h @@ -31,6 +31,8 @@ void subsystems_prefork(void); void subsystems_postfork(void); void subsystems_thread_cleanup(void); +void subsystems_dump_list(void); + struct config_mgr_t; int subsystems_register_options_formats(struct config_mgr_t *mgr); int subsystems_register_state_formats(struct config_mgr_t *mgr); diff --git a/src/core/mainloop/mainloop_sys.c b/src/core/mainloop/mainloop_sys.c index 4b78c90b96..884bae1c59 100644 --- a/src/core/mainloop/mainloop_sys.c +++ b/src/core/mainloop/mainloop_sys.c @@ -78,6 +78,7 @@ mainloop_flush_state(void *arg) const struct subsys_fns_t sys_mainloop = { .name = "mainloop", + SUBSYS_DECLARE_LOCATION(), .supported = true, .level = 5, .initialize = subsys_mainloop_initialize, diff --git a/src/core/or/or_sys.c b/src/core/or/or_sys.c index 126f5448cf..73c6087dce 100644 --- a/src/core/or/or_sys.c +++ b/src/core/or/or_sys.c @@ -47,6 +47,7 @@ subsys_or_add_pubsub(struct pubsub_connector_t *connector) const struct subsys_fns_t sys_or = { .name = "or", + SUBSYS_DECLARE_LOCATION(), .supported = true, .level = 20, .initialize = subsys_or_initialize, diff --git a/src/feature/control/btrack.c b/src/feature/control/btrack.c index 3595af0fcc..405630ecd4 100644 --- a/src/feature/control/btrack.c +++ b/src/feature/control/btrack.c @@ -56,6 +56,7 @@ btrack_add_pubsub(pubsub_connector_t *connector) const subsys_fns_t sys_btrack = { .name = "btrack", + SUBSYS_DECLARE_LOCATION(), .supported = true, .level = 55, .initialize = btrack_init, diff --git a/src/feature/dirauth/dirauth_stub.c b/src/feature/dirauth/dirauth_stub.c index 15a195b0fb..9f48ce14fd 100644 --- a/src/feature/dirauth/dirauth_stub.c +++ b/src/feature/dirauth/dirauth_stub.c @@ -26,6 +26,7 @@ static const config_format_t dirauth_options_stub_fmt = { const struct subsys_fns_t sys_dirauth = { .name = "dirauth", + SUBSYS_DECLARE_LOCATION(), .supported = false, .level = DIRAUTH_SUBSYS_LEVEL, diff --git a/src/feature/dirauth/dirauth_sys.c b/src/feature/dirauth/dirauth_sys.c index 56ac501e16..07c5743877 100644 --- a/src/feature/dirauth/dirauth_sys.c +++ b/src/feature/dirauth/dirauth_sys.c @@ -60,6 +60,7 @@ dirauth_set_options(void *arg) const struct subsys_fns_t sys_dirauth = { .name = "dirauth", + SUBSYS_DECLARE_LOCATION(), .supported = true, .level = DIRAUTH_SUBSYS_LEVEL, .initialize = subsys_dirauth_initialize, diff --git a/src/feature/relay/relay_stub.c b/src/feature/relay/relay_stub.c index 42e08fcb6c..283aaf6e49 100644 --- a/src/feature/relay/relay_stub.c +++ b/src/feature/relay/relay_stub.c @@ -15,6 +15,7 @@ const struct subsys_fns_t sys_relay = { .name = "relay", + SUBSYS_DECLARE_LOCATION(), .supported = false, .level = RELAY_SUBSYS_LEVEL, }; diff --git a/src/feature/relay/relay_sys.c b/src/feature/relay/relay_sys.c index 34489cf5aa..2e90740925 100644 --- a/src/feature/relay/relay_sys.c +++ b/src/feature/relay/relay_sys.c @@ -41,6 +41,7 @@ subsys_relay_shutdown(void) const struct subsys_fns_t sys_relay = { .name = "relay", + SUBSYS_DECLARE_LOCATION(), .supported = true, .level = RELAY_SUBSYS_LEVEL, .initialize = subsys_relay_initialize, diff --git a/src/lib/compress/compress.c b/src/lib/compress/compress.c index 84e9601920..7ce3910d84 100644 --- a/src/lib/compress/compress.c +++ b/src/lib/compress/compress.c @@ -694,6 +694,7 @@ subsys_compress_initialize(void) const subsys_fns_t sys_compress = { .name = "compress", + SUBSYS_DECLARE_LOCATION(), .supported = true, .level = -55, .initialize = subsys_compress_initialize, diff --git a/src/lib/crypt_ops/crypto_init.c b/src/lib/crypt_ops/crypto_init.c index f09bf07c4d..a836bd8645 100644 --- a/src/lib/crypt_ops/crypto_init.c +++ b/src/lib/crypt_ops/crypto_init.c @@ -317,6 +317,7 @@ crypto_set_options(void *arg) const struct subsys_fns_t sys_crypto = { .name = "crypto", + SUBSYS_DECLARE_LOCATION(), .supported = true, .level = -60, .initialize = subsys_crypto_initialize, diff --git a/src/lib/err/torerr_sys.c b/src/lib/err/torerr_sys.c index 46fc853550..8ee1521f3b 100644 --- a/src/lib/err/torerr_sys.c +++ b/src/lib/err/torerr_sys.c @@ -34,6 +34,7 @@ subsys_torerr_shutdown(void) const subsys_fns_t sys_torerr = { .name = "err", + SUBSYS_DECLARE_LOCATION(), /* Low-level error handling is a diagnostic feature, we want it to init * right after windows process security, and shutdown last. * (Security never shuts down.) */ diff --git a/src/lib/evloop/evloop_sys.c b/src/lib/evloop/evloop_sys.c index fecec2f264..b639810c23 100644 --- a/src/lib/evloop/evloop_sys.c +++ b/src/lib/evloop/evloop_sys.c @@ -41,6 +41,7 @@ subsys_evloop_shutdown(void) const struct subsys_fns_t sys_evloop = { .name = "evloop", + SUBSYS_DECLARE_LOCATION(), .supported = true, .level = -20, .initialize = subsys_evloop_initialize, diff --git a/src/lib/llharden/winprocess_sys.c b/src/lib/llharden/winprocess_sys.c index a5f22c182b..f2c88d8c75 100644 --- a/src/lib/llharden/winprocess_sys.c +++ b/src/lib/llharden/winprocess_sys.c @@ -58,6 +58,7 @@ subsys_winprocess_initialize(void) const subsys_fns_t sys_winprocess = { .name = "winprocess", + SUBSYS_DECLARE_LOCATION(), /* HeapEnableTerminationOnCorruption and setdeppolicy() are security * features, we want them to run first. */ .level = -100, diff --git a/src/lib/log/log_sys.c b/src/lib/log/log_sys.c index 1be4f5b7d8..021c05d3e6 100644 --- a/src/lib/log/log_sys.c +++ b/src/lib/log/log_sys.c @@ -28,6 +28,7 @@ subsys_logging_shutdown(void) const subsys_fns_t sys_logging = { .name = "log", + SUBSYS_DECLARE_LOCATION(), .supported = true, /* Logging depends on threads, approx time, raw logging, and security. * Most other lib modules depend on logging. */ diff --git a/src/lib/net/network_sys.c b/src/lib/net/network_sys.c index f0421385b7..e95c3ba819 100644 --- a/src/lib/net/network_sys.c +++ b/src/lib/net/network_sys.c @@ -37,6 +37,7 @@ subsys_network_shutdown(void) const subsys_fns_t sys_network = { .name = "network", + SUBSYS_DECLARE_LOCATION(), /* Network depends on logging, and a lot of other modules depend on network. */ .level = -55, diff --git a/src/lib/process/process_sys.c b/src/lib/process/process_sys.c index 015ffadead..c8332ba91e 100644 --- a/src/lib/process/process_sys.c +++ b/src/lib/process/process_sys.c @@ -26,6 +26,7 @@ subsys_process_shutdown(void) const subsys_fns_t sys_process = { .name = "process", + SUBSYS_DECLARE_LOCATION(), .level = -18, .supported = true, .initialize = subsys_process_initialize, diff --git a/src/lib/subsys/subsys.h b/src/lib/subsys/subsys.h index c05b69af39..b29015746e 100644 --- a/src/lib/subsys/subsys.h +++ b/src/lib/subsys/subsys.h @@ -42,6 +42,11 @@ typedef struct subsys_fns_t { const char *name; /** + * The file in which the subsystem object is declared. Used for debugging. + **/ + const char *location; + + /** * Whether this subsystem is supported -- that is, whether it is compiled * into Tor. For most subsystems, this should be true. **/ @@ -187,6 +192,14 @@ typedef struct subsys_fns_t { int (*flush_state)(void *); } subsys_fns_t; +#ifndef COCCI +/** + * Macro to declare a subsystem's location. + **/ +#define SUBSYS_DECLARE_LOCATION() \ + .location = __FILE__ +#endif + /** * Lowest allowed subsystem level. **/ diff --git a/src/lib/thread/compat_threads.c b/src/lib/thread/compat_threads.c index 21125bddad..75ade9c9f2 100644 --- a/src/lib/thread/compat_threads.c +++ b/src/lib/thread/compat_threads.c @@ -129,6 +129,7 @@ subsys_threads_initialize(void) const subsys_fns_t sys_threads = { .name = "threads", + SUBSYS_DECLARE_LOCATION(), .supported = true, .level = -89, .initialize = subsys_threads_initialize, diff --git a/src/lib/time/time_sys.c b/src/lib/time/time_sys.c index 044d328f81..1c1bc4cd18 100644 --- a/src/lib/time/time_sys.c +++ b/src/lib/time/time_sys.c @@ -20,6 +20,7 @@ subsys_time_initialize(void) const subsys_fns_t sys_time = { .name = "time", + SUBSYS_DECLARE_LOCATION(), /* Monotonic time depends on logging, and a lot of other modules depend on * monotonic time. */ .level = -80, diff --git a/src/lib/tls/tortls.c b/src/lib/tls/tortls.c index fd41a84cfa..9e70e54725 100644 --- a/src/lib/tls/tortls.c +++ b/src/lib/tls/tortls.c @@ -456,6 +456,7 @@ subsys_tortls_shutdown(void) const subsys_fns_t sys_tortls = { .name = "tortls", + SUBSYS_DECLARE_LOCATION(), .level = -50, .shutdown = subsys_tortls_shutdown }; diff --git a/src/lib/wallclock/approx_time.c b/src/lib/wallclock/approx_time.c index d9f90ab2f7..c815f20e51 100644 --- a/src/lib/wallclock/approx_time.c +++ b/src/lib/wallclock/approx_time.c @@ -59,6 +59,7 @@ subsys_wallclock_initialize(void) **/ const subsys_fns_t sys_wallclock = { .name = "wallclock", + SUBSYS_DECLARE_LOCATION(), .supported = true, /* Approximate time is a diagnostic feature, we want it to init right after * low-level error handling. */ diff --git a/src/test/include.am b/src/test/include.am index de927836d6..e7647260c5 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -37,7 +37,8 @@ TESTSCRIPTS += \ src/test/test_ntor.sh \ src/test/test_hs_ntor.sh \ src/test/test_bt.sh \ - scripts/maint/practracker/test_practracker.sh + scripts/maint/practracker/test_practracker.sh \ + scripts/maint/run_check_subsystem_order.sh if COVERAGE_ENABLED # ... @@ -430,6 +431,7 @@ EXTRA_DIST += \ src/test/test_rebind.sh \ src/test/test_rebind.py \ src/test/zero_length_keys.sh \ + scripts/maint/run_check_subsystem_order.sh \ src/test/rust_supp.txt \ src/test/test_keygen.sh \ src/test/test_key_expiration.sh \ |