diff --git a/Makefile.am b/Makefile.am index e2321bb1..c8618a84 100644 --- a/Makefile.am +++ b/Makefile.am @@ -224,6 +224,8 @@ EXTRA_DIST = $(DOC_FILES) $(man_MANS) $(TESTS) $(TEST_LOG_COMPILER) \ libjq.pc \ tests/modules/a.jq tests/modules/b/b.jq tests/modules/c/c.jq \ tests/modules/c/d.jq tests/modules/data.json \ + tests/modules/deep1.jq tests/modules/deep2a.jq \ + tests/modules/deep2b.jq tests/modules/deep3.jq \ tests/modules/home1/.jq tests/modules/home2/.jq/g.jq \ tests/modules/lib/jq/e/e.jq tests/modules/lib/jq/f.jq \ tests/modules/shadow1.jq tests/modules/shadow2.jq \ diff --git a/src/linker.c b/src/linker.c index 03f46db0..25833362 100644 --- a/src/linker.c +++ b/src/linker.c @@ -23,11 +23,15 @@ struct lib_entry { char *name; block def; - int loading; +}; +struct loading_lib { + const char *name; + struct loading_lib *next; }; struct lib_loading_state { struct lib_entry *entries; uint64_t ct; + struct loading_lib *loading; }; static int load_library(jq_state *jq, jv lib_path, int is_data, int raw, int optional, @@ -312,16 +316,6 @@ static int process_dependencies(jq_state *jq, jv jq_origin, jv lib_origin, block } if (state_idx < lib_state->ct) { // Found - if (lib_state->entries[state_idx].loading) { - jq_report_error(jq, jv_string_fmt("jq: error: circular import of %s\n", - jv_string_value(resolved))); - jv_free(resolved); - jv_free(as); - jv_free(deps); - jv_free(jq_origin); - jv_free(lib_origin); - return 1; - } jv_free(resolved); // Bind the library to the program bk = block_bind_library(lib_state->entries[state_idx].def, bk, OP_IS_CALL_PSEUDO, as_str); @@ -351,11 +345,20 @@ static int load_library(jq_state *jq, jv lib_path, int is_data, int raw, int opt struct locfile* src = NULL; block program; jv data; + for (struct loading_lib *l = lib_state->loading; l; l = l->next) { + if (strcmp(l->name, jv_string_value(lib_path)) == 0) { + jq_report_error(jq, jv_string_fmt("jq: error: circular import of %s\n", + jv_string_value(lib_path))); + *out_block = gen_noop(); + jv_free(lib_path); + return 1; + } + } if (is_data && !raw) data = jv_load_file(jv_string_value(lib_path), 0); else data = jv_load_file(jv_string_value(lib_path), 1); - int state_idx; + uint64_t state_idx; if (!jv_is_valid(data)) { program = gen_noop(); if (!optional) { @@ -373,29 +376,33 @@ static int load_library(jq_state *jq, jv lib_path, int is_data, int raw, int opt lib_state->entries = jv_mem_realloc(lib_state->entries, lib_state->ct * sizeof(struct lib_entry)); lib_state->entries[state_idx].name = strdup(jv_string_value(lib_path)); lib_state->entries[state_idx].def = program; - lib_state->entries[state_idx].loading = 0; } else { // import "foo" as bar; src = locfile_init(jq, jv_string_value(lib_path), jv_string_value(data), jv_string_length_bytes(jv_copy(data))); nerrors += jq_parse_library(src, &program); locfile_free(src); if (nerrors == 0) { - // Register the library before processing its dependencies so that - // circular imports can be detected. - state_idx = lib_state->ct++; - lib_state->entries = jv_mem_realloc(lib_state->entries, lib_state->ct * sizeof(struct lib_entry)); - lib_state->entries[state_idx].name = strdup(jv_string_value(lib_path)); - lib_state->entries[state_idx].def = gen_noop(); - lib_state->entries[state_idx].loading = 1; - + // Mark the library as being loaded while its dependencies are processed, + // so that a recursive reference to it is reported as a circular import. + // The node lives on this frame and is popped before lib_path is freed, + // so it can borrow the path string. + struct loading_lib loading = { jv_string_value(lib_path), lib_state->loading }; + lib_state->loading = &loading; char *lib_origin = strdup(jv_string_value(lib_path)); nerrors += process_dependencies(jq, jq_get_jq_origin(jq), jv_string(dirname(lib_origin)), &program, lib_state); free(lib_origin); + lib_state->loading = loading.next; program = block_bind_self(program, OP_IS_CALL_PSEUDO); + // Register the library only once its dependencies have been processed, + // to keep lib_state->entries in dependency order. Register it even when + // its dependencies failed, so that other importers reuse this entry + // rather than loading and diagnosing the same library again. + state_idx = lib_state->ct++; + lib_state->entries = jv_mem_realloc(lib_state->entries, lib_state->ct * sizeof(struct lib_entry)); + lib_state->entries[state_idx].name = strdup(jv_string_value(lib_path)); lib_state->entries[state_idx].def = program; - lib_state->entries[state_idx].loading = 0; } } *out_block = program; @@ -435,7 +442,7 @@ jv load_module_meta(jq_state *jq, jv mod_relpath) { int load_program(jq_state *jq, struct locfile* src, block *out_block) { int nerrors = 0; block program; - struct lib_loading_state lib_state = {0,0}; + struct lib_loading_state lib_state = {0,0,0}; nerrors = jq_parse(src, &program); if (nerrors) return nerrors; diff --git a/tests/jq.test b/tests/jq.test index 20dfeb11..ff3c625b 100644 --- a/tests/jq.test +++ b/tests/jq.test @@ -1950,6 +1950,23 @@ import "shadow1" as f; import "shadow2" as f; import "shadow1" as e; [e::e, f::e null [2,3] +# A module reached only through another module must be linked before its +# importers, or dead-code elimination drops the definitions that only the +# importer's body references (#3597) +include "deep1"; f +null +[1,[21,3],[22,3,1]] + +# ... including when the shared dependency is pulled in first by an importer +# whose own definitions end up unused, in either import order +import "deep2a" as a; import "deep2b" as b; b::f +null +[22,3,1] + +import "deep2b" as b; import "deep2a" as a; b::f +null +[22,3,1] + %%FAIL module (.+1); 0 jq: error: Module metadata must be constant at , line 1, column 8: diff --git a/tests/modules/deep1.jq b/tests/modules/deep1.jq new file mode 100644 index 00000000..d10e3139 --- /dev/null +++ b/tests/modules/deep1.jq @@ -0,0 +1,3 @@ +import "deep2a" as a; +import "deep2b" as b; +def f: [1, a::f, b::f]; diff --git a/tests/modules/deep2a.jq b/tests/modules/deep2a.jq new file mode 100644 index 00000000..b9a2718c --- /dev/null +++ b/tests/modules/deep2a.jq @@ -0,0 +1,2 @@ +import "deep3" as m; +def f: [21, m::f]; diff --git a/tests/modules/deep2b.jq b/tests/modules/deep2b.jq new file mode 100644 index 00000000..e289e41b --- /dev/null +++ b/tests/modules/deep2b.jq @@ -0,0 +1,3 @@ +import "deep3" as m; +import "data" as $d; +def f: [22, m::f, ($d | length)]; diff --git a/tests/modules/deep3.jq b/tests/modules/deep3.jq new file mode 100644 index 00000000..bf9cd254 --- /dev/null +++ b/tests/modules/deep3.jq @@ -0,0 +1,2 @@ +def _f: 3; +def f: _f;