From 53d74c0954b989d478c868b12647e1088a079457 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 14 Feb 2020 09:39:36 -0500 Subject: [PATCH 01/12] Add a "location" field for subsystems to declare which file they are in. --- src/lib/subsys/subsys.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) 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 @@ -41,6 +41,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. **/ From 3f6e37b1bcb9e5ec70f228a9cb6b01db0b9ad8d5 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 14 Feb 2020 09:57:46 -0500 Subject: [PATCH 02/12] Add a SUBSYS_DECLARE_LOCATION() to every subsystem. --- src/core/mainloop/mainloop_sys.c | 1 + src/core/or/or_sys.c | 1 + src/feature/control/btrack.c | 1 + src/feature/dirauth/dirauth_stub.c | 1 + src/feature/dirauth/dirauth_sys.c | 1 + src/feature/relay/relay_stub.c | 1 + src/feature/relay/relay_sys.c | 1 + src/lib/compress/compress.c | 1 + src/lib/crypt_ops/crypto_init.c | 1 + src/lib/err/torerr_sys.c | 1 + src/lib/evloop/evloop_sys.c | 1 + src/lib/llharden/winprocess_sys.c | 1 + src/lib/log/log_sys.c | 1 + src/lib/net/network_sys.c | 1 + src/lib/process/process_sys.c | 1 + src/lib/thread/compat_threads.c | 1 + src/lib/time/time_sys.c | 1 + src/lib/tls/tortls.c | 1 + src/lib/wallclock/approx_time.c | 1 + 19 files changed, 19 insertions(+) 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/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. */ From 8f49943459564a59ca7f60476aef6b0ddd79a56c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 30 Sep 2019 10:05:42 -0400 Subject: [PATCH 03/12] Implement a function to list the subsystems to stdout. --- src/app/main/subsysmgr.c | 13 +++++++++++++ src/app/main/subsysmgr.h | 2 ++ 2 files changed, 15 insertions(+) diff --git a/src/app/main/subsysmgr.c b/src/app/main/subsysmgr.c index 5807cbbaa4..4913949e2c 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -293,6 +293,19 @@ 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%s\n", sys->level, sys->name); + } +} + /** * Register all subsystem-declared options formats in mgr. * 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); From ac72ecd5813e878e223df48556a43061e0d0068c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 30 Sep 2019 10:07:20 -0400 Subject: [PATCH 04/12] Add --dbg-dump-subsystem-list command to list the subsystems. I'm prefixing this with --dbg-* because it is not meant to be used externally. --- src/app/config/config.c | 7 +++++++ src/app/main/subsysmgr.c | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) 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 4913949e2c..de601d28cd 100644 --- a/src/app/main/subsysmgr.c +++ b/src/app/main/subsysmgr.c @@ -302,7 +302,8 @@ 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%s\n", sys->level, sys->name); + printf("% 4d\t%16s\t%s\n", sys->level, sys->name, + sys->location?sys->location:""); } } From c323cc0913bae31a0b70c94bf5fd685acd3d1c07 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 30 Sep 2019 10:10:50 -0400 Subject: [PATCH 05/12] Document the --dbg- prefix in tor.1 --- doc/tor.1.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index ba9b9bb5e0..06772a5c7f 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 From a40d539f7cce851a4a66c68a14917e1681972475 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 14 Feb 2020 12:58:39 -0500 Subject: [PATCH 06/12] Extend includes.py to compare topology with subsystem init order. --- scripts/maint/practracker/includes.py | 50 +++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/scripts/maint/practracker/includes.py b/scripts/maint/practracker/includes.py index e9b02c35b0..7af378d5c0 100755 --- a/scripts/maint/practracker/includes.py +++ b/scripts/maint/practracker/includes.py @@ -173,6 +173,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 +254,25 @@ def walk_c_files(topdir="src"): for err in consider_include_rules(fullpath, f): yield err +def check_subsys_file(fname, uses_dirs): + uses_closure = closure(uses_dirs) + ok = True + previous_subsystems = [] + with open(fname) as f: + for line in f: + _, name, fname = line.split() + fname = re.sub(r'^.*/src/', "", fname) + fname = re.sub(r'^src/', "", fname) + fname = re.sub(r'/[^/]*\.c', "", 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 +297,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,6 +325,8 @@ 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:]) @@ -289,7 +334,8 @@ def main(argv): 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) From 978b7ef45c21169a01a81cd1b206530cbcd47fe2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Mar 2020 12:30:30 -0400 Subject: [PATCH 07/12] Teach --check-subsystem-order to take input from stdin --- scripts/maint/practracker/includes.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/scripts/maint/practracker/includes.py b/scripts/maint/practracker/includes.py index 7af378d5c0..7d70a6a0aa 100755 --- a/scripts/maint/practracker/includes.py +++ b/scripts/maint/practracker/includes.py @@ -254,11 +254,18 @@ 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): uses_closure = closure(uses_dirs) ok = True previous_subsystems = [] - with open(fname) as f: + + with open_or_stdin(fname) as f: for line in f: _, name, fname = line.split() fname = re.sub(r'^.*/src/', "", fname) From 9b434b79ce74dab0498fc0ee1df5df3b2441cfad Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Mar 2020 12:37:49 -0400 Subject: [PATCH 08/12] Add a test script to check subsystem order as part of make check. --- scripts/maint/run_check_subsystem_order.sh | 17 +++++++++++++++++ src/test/include.am | 4 +++- 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100755 scripts/maint/run_check_subsystem_order.sh diff --git a/scripts/maint/run_check_subsystem_order.sh b/scripts/maint/run_check_subsystem_order.sh new file mode 100755 index 0000000000..1e68b9d008 --- /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 - + +echo ok 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 \ From f9c57313d03abccbbbcffc808464365500baced8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Mar 2020 12:40:56 -0400 Subject: [PATCH 09/12] add changes file for 31634 --- changes/ticket31634 | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changes/ticket31634 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. From 5f49695f94abd089dfbaa2e219ac277aa5c15242 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 4 Apr 2020 11:31:30 -0400 Subject: [PATCH 10/12] Make check_subsystem_order work in out-of-tree builds. --- scripts/maint/practracker/includes.py | 11 +++++++++++ scripts/maint/run_check_subsystem_order.sh | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/scripts/maint/practracker/includes.py b/scripts/maint/practracker/includes.py index 7d70a6a0aa..e08fd0d706 100755 --- a/scripts/maint/practracker/includes.py +++ b/scripts/maint/practracker/includes.py @@ -136,6 +136,15 @@ class Rules(object): return allowed +def normalize_srcdir(fname): + """given the name of a source directory or file, return its name + relative to `src`. + """ + fname = re.sub(r'^.*/src/', "", fname) + fname = re.sub(r'^src/', "", fname) + fname = re.sub(r'/[^/]*\.[ch]', "", fname) + return fname + include_rules_cache = {} def load_include_rules(fname): @@ -261,6 +270,8 @@ def open_or_stdin(fname): return open(fname) def check_subsys_file(fname, uses_dirs): + 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 = [] diff --git a/scripts/maint/run_check_subsystem_order.sh b/scripts/maint/run_check_subsystem_order.sh index 1e68b9d008..4ec73bfd56 100755 --- a/scripts/maint/run_check_subsystem_order.sh +++ b/scripts/maint/run_check_subsystem_order.sh @@ -12,6 +12,6 @@ if ! test -x "${INCLUDES_PY}" ; then fi "${TOR}" --dbg-dump-subsystem-list | \ - "${INCLUDES_PY}" --check-subsystem-order - + "${INCLUDES_PY}" --check-subsystem-order - "${abs_top_srcdir}/src" echo ok From d3db84c1e7792e75f75d3fcbf66ba1bbd7617aa9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 4 Apr 2020 11:31:30 -0400 Subject: [PATCH 11/12] Make check_subsystem_order work in distcheck builds. --- scripts/maint/practracker/includes.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/maint/practracker/includes.py b/scripts/maint/practracker/includes.py index e08fd0d706..ff465bde63 100755 --- a/scripts/maint/practracker/includes.py +++ b/scripts/maint/practracker/includes.py @@ -270,6 +270,11 @@ def open_or_stdin(fname): 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. + 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) From ea9ff58921da4f636a3c5faf409224cdd13c4a8a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 6 Apr 2020 10:48:59 -0400 Subject: [PATCH 12/12] Try rewriting normalize_srcdir to normalize harder. --- scripts/maint/practracker/includes.py | 33 ++++++++++++++++++++------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/scripts/maint/practracker/includes.py b/scripts/maint/practracker/includes.py index ff465bde63..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,14 +138,28 @@ class Rules(object): return allowed + def normalize_srcdir(fname): """given the name of a source directory or file, return its name - relative to `src`. + relative to `src` in a unix-like format. """ - fname = re.sub(r'^.*/src/', "", fname) - fname = re.sub(r'^src/', "", fname) - fname = re.sub(r'/[^/]*\.[ch]', "", fname) - return fname + 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 = {} @@ -273,6 +289,7 @@ 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 } @@ -284,9 +301,7 @@ def check_subsys_file(fname, uses_dirs): with open_or_stdin(fname) as f: for line in f: _, name, fname = line.split() - fname = re.sub(r'^.*/src/', "", fname) - fname = re.sub(r'^src/', "", fname) - fname = re.sub(r'/[^/]*\.c', "", fname) + fname = normalize_srcdir(fname) for prev in previous_subsystems: if fname in uses_closure[prev]: print("INVERSION: {} uses {}".format(prev,fname)) @@ -354,6 +369,8 @@ def main(argv): 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,