Merge remote-tracking branch 'origin/topic/bbannier/issue-1624'

zeek · Feb 27, 2024 · 031ca00 · 031ca00
2 parents 349674d + 7c5d58b
commit 031ca00
Show file tree

Hide file tree

Showing 14 changed files with 85 additions and 150 deletions.
diff --git a/CHANGES b/CHANGES
@@ -1,3 +1,42 @@
+1.10.0-dev.156 | 2024-02-27 17:44:22 +0100
+
+  * GH-1624: Enable optimizations when running `spicy-build`. (Benjamin Bannier, Corelight)
+
+    We previously would emit single C++ source files from `spicy-build`.
+    This made it impossible to enable optimizations since we could not know
+    whether individual code was used. Since optional features are by default
+    enabled and only disabled through an optimizer pass this meant that
+    `spicy-build` emitted code which performed a lot of work not done
+    usually.
+
+    This patch reworks `spicy-build` to use `spicyc`'s `-x` instead which
+    has a global view and can run optimizations, and emits all source files
+    into a prefix.
+
+    Closes #1624.
+    Closes #1625.
+    Closes #1622.
+
+  * Fix some shellcheck warnings in `spicy-build`. (Benjamin Bannier, Corelight)
+
+  * Fix handling of `spicyc -x` when passed a filename. (Benjamin Bannier, Corelight)
+
+    For the `-x` flag users can pass either the name of a directory or a
+    file. If passed a filename the file name is incorporated into the
+    identifier generated namespace identifier of e.g., the form
+    `__hlt_FILENAME` in the emitted C++ files. Previously when passing a
+    directory we would still emit a `_` separator, but since there was no
+    filename nothing after it so we generated namespace identifiers like
+    `__hlt_`.
+
+    With this patch we now only emit the separator if passed a filename.
+
+  * Do not optimize out public functions. (Benjamin Bannier, Corelight)
+
+    Optimizing out public functions breaks the assumption we have elsewhere
+    that something marked `public` is part of the public API and would not
+    get optimized out. This is e.g., the behavior we have for units.
+
 1.10.0-dev.151 | 2024-02-26 12:16:17 +0100
 
   * Fix stray Python escape sequence. (Benjamin Bannier, Corelight)

diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-1.10.0-dev.151
+1.10.0-dev.156
diff --git a/hilti/toolchain/src/compiler/driver.cc b/hilti/toolchain/src/compiler/driver.cc
@@ -285,10 +285,11 @@ Result<Nothing> Driver::parseOptions(int argc, char** argv) {
                 _driver_options.output_cxx_prefix = optarg;
                 _driver_options.execute_code = false;
                 _driver_options.include_linker = true;
-                _compiler_options.cxx_namespace_extern =
-                    hilti::util::fmt("hlt_%s", hilti::rt::filesystem::path(optarg).stem().string());
-                _compiler_options.cxx_namespace_intern =
-                    hilti::util::fmt("__hlt_%s", hilti::rt::filesystem::path(optarg).stem().string());
+
+                if ( auto s = hilti::rt::filesystem::path(optarg).stem().string(); ! s.empty() ) {
+                    _compiler_options.cxx_namespace_extern = hilti::util::fmt("hlt_%s", s);
+                    _compiler_options.cxx_namespace_intern = hilti::util::fmt("__hlt_%s", s);
+                }
 
                 ++num_output_types;
                 break;

diff --git a/hilti/toolchain/src/compiler/optimizer.cc b/hilti/toolchain/src/compiler/optimizer.cc
@@ -2,7 +2,6 @@
 
 #include "hilti/compiler/optimizer.h"
 
-#include <algorithm>
 #include <optional>
 #include <string>
 #include <tuple>
@@ -12,6 +11,7 @@
 
 #include <hilti/ast/builder/expression.h>
 #include <hilti/ast/ctors/default.h>
+#include <hilti/ast/declaration.h>
 #include <hilti/ast/declarations/function.h>
 #include <hilti/ast/declarations/imported-module.h>
 #include <hilti/ast/detail/visitor.h>
@@ -274,7 +274,10 @@ struct FunctionVisitor : OptimizerVisitor, visitor::PreOrder<bool, FunctionVisit
                 // If the function declaration is marked `&always-emit` mark it as referenced.
                 bool is_always_emit = AttributeSet::find(fn.attributes(), "&always-emit").has_value();
 
-                if ( is_always_emit )
+                // If the function is public mark is as referenced.
+                bool is_public = x.linkage() == declaration::Linkage::Public;
+
+                if ( is_always_emit || is_public )
                     function.referenced = true;
 
                 // For implementation of methods check whether the method

diff --git a/spicy/toolchain/bin/spicy-build b/spicy/toolchain/bin/spicy-build
@@ -1,10 +1,10 @@
-#! /bin/sh
+#!/bin/sh
 #
 # Copyright (c) 2020-2023 by the Zeek Project. See LICENSE for details.
 
 usage() {
     cat << EOF
-$(basename $0) [options] <input files>
+$(basename "$0") [options] <input files>
 
     -d          Build a debug version.
     -g          Disable HILTI-side optimizations of the generated code.
@@ -19,26 +19,26 @@ EOF
 }
 
 mktmp() {
-    base=$(basename $1)
+    base=$(basename "$1")
     ext=$2
     cnt=0
     while : ; do
         tmp=${dest}/${base}.${cnt}.tmp.${ext}
-        test -e ${tmp} || break
+        test -e "${tmp}" || break
         cnt=$((cnt + 1))
     done
-    touch ${tmp}
-    echo ${tmp}
+    touch "${tmp}"
+    echo "${tmp}"
 }
 
 mkcc() {
-    tmp=$(mktmp $1 cc)
-    echo ${tmp}
+    tmp=$(mktmp "$1" cc)
+    echo "${tmp}"
 }
 
 execute() {
     # Remove absolute path of executable
-    test "${verbose}" = 1 && echo $@ | sed 's#^[^ ]*/\([a-zA-Z+_-]\{1,\}\) #> \1 #'
+    test "${verbose}" = 1 && echo "$@" | sed 's#^[^ ]*/\([a-zA-Z+_-]\{1,\}\) #> \1 #'
     $@
 }
 
@@ -76,13 +76,13 @@ while getopts "ghdo:Stv" opt; do
     esac
 done
 
-shift $((${OPTIND} - 1))
+shift $((OPTIND - 1))
 
-inputs=$@
+inputs=$*
 test -z "${inputs}" && usage && exit 1
 test -z "${out}" && usage && exit 1
 
-base=$(cd $(dirname $0)/.. && pwd)
+base=$(cd "$(dirname "$0")/.." && pwd)
 
 if [ "${out}" != "${out#/}" ]; then
     # absolute path
@@ -98,11 +98,11 @@ mkdir -p "${dest}"
 trap cleanup EXIT
 
 for i in $(which hilti-config 2>/dev/null) ${base}/bin/hilti-config ${base}/build/bin/hilti-config; do
-   test -x $i && hilti_config=$i && break
+   test -x "$i" && hilti_config=$i && break
 done
 
 for i in $(which spicy-config 2>/dev/null) ${base}/bin/spicy-config ${base}/build/bin/spicy-config; do
-   test -x $i && spicy_config=$i && break
+   test -x "$i" && spicy_config=$i && break
 done
 
 if [ -z "${hilti_config}" ]; then
@@ -121,33 +121,31 @@ spicy_lib_dir=$(for i in $(${spicy_config} --libdirs); do test -e "${i}/spicy_rt
 spicy_driver=${spicy_lib_dir}/spicy-driver-host.cc
 
 rt_modules=""
-cxxs=""
 
+cc_inputs=""
 if [ "${compile_spicy_driver}" = 1 ]; then
     rt_modules="${spicy_lib_dir}/filter.spicy"
-    cxxs="${spicy_driver}"
+    cc_inputs="${cc_inputs} ${spicy_driver}"
 fi
 
-n=0
-for i in ${inputs} ${rt_modules}; do
+spicy_inputs=""
+for i in ${inputs}; do
     case "${i}" in
         *.cc)
-            cc=${i}
+            cc_inputs="${cc_inputs} ${i}"
             ;;
         *)
-            # Generate C++ code for HILTI/Spicy source.
-            cc=$(mkcc ${out})
-            execute $(${spicy_config} --spicyc) -g ${hiltic_flags} -c -o ${cc} ${i} || exit 1
+            spicy_inputs="${spicy_inputs} ${i}"
             ;;
     esac
-
-   cxxs="${cxxs} ${cc}"
 done
 
-# Generate C++ linker code
-cc=$(mkcc ${out})
-execute $(${spicy_config} --spicyc) ${hiltic_flags} -g -l -o ${cc} ${cxxs} || exit 1
-cxxs="${cxxs} ${cc}"
+
+# Generate C++ output files. We pass `cc_inputs` here as well even though its
+# only effect is to cause emitting of generated files.
+execute "$("${spicy_config}" --spicyc)" "${hiltic_flags}" -x "${dest}/" "${spicy_inputs}" "${cc_inputs}" "${rt_modules}" || exit 1
+
+cxxs="${cc_inputs} $(find "${dest}" -type f)"
 
 # Compile all C++ code into executable
-execute ${cxx} ${cxxs} ${ldflags} -o ${out} || exit 1
+execute "${cxx}" "${cxxs}" "${ldflags}" -o "${out}" || exit 1
diff --git a/tests/Baseline/hilti.optimization.const/log b/tests/Baseline/hilti.optimization.const/log
@@ -7,13 +7,3 @@
 [debug/optimizer] inlining constant 'Foo::t'
 [debug/optimizer] inlining constant 'Foo::t'
 [debug/optimizer] inlining constant 'Foo::t'
-[debug/optimizer] removing declaration for unused function hilti::abort
-[debug/optimizer] removing declaration for unused function hilti::current_time
-[debug/optimizer] removing declaration for unused function hilti::debug
-[debug/optimizer] removing declaration for unused function hilti::debugDedent
-[debug/optimizer] removing declaration for unused function hilti::debugIndent
-[debug/optimizer] removing declaration for unused function hilti::exception_what
-[debug/optimizer] removing declaration for unused function hilti::mktime
-[debug/optimizer] removing declaration for unused function hilti::printValues
-[debug/optimizer] removing declaration for unused function hilti::profiler_start
-[debug/optimizer] removing declaration for unused function hilti::profiler_stop
diff --git a/tests/Baseline/hilti.optimization.unused_function/log b/tests/Baseline/hilti.optimization.unused_function/log
@@ -1,4 +1,3 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
 [debug/optimizer] removing declaration for unused function Foo::private_unused
 [debug/optimizer] removing declaration for unused function Foo::private_unused1
-[debug/optimizer] removing declaration for unused function Foo::public_unused
diff --git a/tests/Baseline/hilti.optimization.unused_function/opt.hlt b/tests/Baseline/hilti.optimization.unused_function/opt.hlt
@@ -7,6 +7,10 @@ function bool private_used() {
     return False;
 }
 
+public function bool public_unused() {
+    return False;
+}
+
 public function extern bool extern_unused1() {
     return False;
 }

diff --git a/tests/Baseline/spicy.optimization.default-parser-functions/log b/tests/Baseline/spicy.optimization.default-parser-functions/log
@@ -137,30 +137,6 @@
 [debug/optimizer] removing declaration for unused function foo::P0::parse1
 [debug/optimizer] removing declaration for unused function foo::P0::parse2
 [debug/optimizer] removing declaration for unused function foo::P0::parse3
-[debug/optimizer] removing declaration for unused function hilti::abort
-[debug/optimizer] removing declaration for unused function hilti::current_time
-[debug/optimizer] removing declaration for unused function hilti::debug
-[debug/optimizer] removing declaration for unused function hilti::mktime
-[debug/optimizer] removing declaration for unused function hilti::print
-[debug/optimizer] removing declaration for unused function hilti::printValues
-[debug/optimizer] removing declaration for unused function hilti::profiler_start
-[debug/optimizer] removing declaration for unused function hilti::profiler_stop
-[debug/optimizer] removing declaration for unused function spicy_rt::atEod
-[debug/optimizer] removing declaration for unused function spicy_rt::backtrack
-[debug/optimizer] removing declaration for unused function spicy_rt::confirm
-[debug/optimizer] removing declaration for unused function spicy_rt::createContext
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_connect
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_disconnect
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_forward
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_forward_eod
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_init
-[debug/optimizer] removing declaration for unused function spicy_rt::printParserState
-[debug/optimizer] removing declaration for unused function spicy_rt::reject
-[debug/optimizer] removing declaration for unused function spicy_rt::setContext
-[debug/optimizer] removing declaration for unused function spicy_rt::unit_find
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForEod
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInputOrEod
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInputOrEod
 [debug/optimizer] removing field for unused method foo::P0::__on_0x25_confirmed
 [debug/optimizer] removing field for unused method foo::P0::__on_0x25_done
 [debug/optimizer] removing field for unused method foo::P0::__on_0x25_error

diff --git a/tests/Baseline/spicy.optimization.feature_requirements/log b/tests/Baseline/spicy.optimization.feature_requirements/log
@@ -351,29 +351,6 @@
 [debug/optimizer] removing declaration for unused function foo::X7::parse1
 [debug/optimizer] removing declaration for unused function foo::X7::parse2
 [debug/optimizer] removing declaration for unused function foo::X7::parse3
-[debug/optimizer] removing declaration for unused function hilti::abort
-[debug/optimizer] removing declaration for unused function hilti::current_time
-[debug/optimizer] removing declaration for unused function hilti::debug
-[debug/optimizer] removing declaration for unused function hilti::exception_what
-[debug/optimizer] removing declaration for unused function hilti::mktime
-[debug/optimizer] removing declaration for unused function hilti::print
-[debug/optimizer] removing declaration for unused function hilti::printValues
-[debug/optimizer] removing declaration for unused function hilti::profiler_start
-[debug/optimizer] removing declaration for unused function hilti::profiler_stop
-[debug/optimizer] removing declaration for unused function spicy_rt::atEod
-[debug/optimizer] removing declaration for unused function spicy_rt::backtrack
-[debug/optimizer] removing declaration for unused function spicy_rt::confirm
-[debug/optimizer] removing declaration for unused function spicy_rt::createContext
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_forward
-[debug/optimizer] removing declaration for unused function spicy_rt::printParserState
-[debug/optimizer] removing declaration for unused function spicy_rt::reject
-[debug/optimizer] removing declaration for unused function spicy_rt::setContext
-[debug/optimizer] removing declaration for unused function spicy_rt::unit_find
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForEod
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInput
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInput
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInputOrEod
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInputOrEod
 [debug/optimizer] removing field for unused method foo::X0::__on_0x25_confirmed
 [debug/optimizer] removing field for unused method foo::X0::__on_0x25_done
 [debug/optimizer] removing field for unused method foo::X0::__on_0x25_error

diff --git a/tests/Baseline/spicy.optimization.unused-functions/log b/tests/Baseline/spicy.optimization.unused-functions/log
@@ -219,34 +219,6 @@
 [debug/optimizer] removing declaration for unused function foo::F::parse2
 [debug/optimizer] removing declaration for unused function foo::F::parse3
 [debug/optimizer] removing declaration for unused function foo::f1
-[debug/optimizer] removing declaration for unused function foo::f2
-[debug/optimizer] removing declaration for unused function hilti::abort
-[debug/optimizer] removing declaration for unused function hilti::current_time
-[debug/optimizer] removing declaration for unused function hilti::debug
-[debug/optimizer] removing declaration for unused function hilti::exception_what
-[debug/optimizer] removing declaration for unused function hilti::mktime
-[debug/optimizer] removing declaration for unused function hilti::print
-[debug/optimizer] removing declaration for unused function hilti::printValues
-[debug/optimizer] removing declaration for unused function hilti::profiler_start
-[debug/optimizer] removing declaration for unused function hilti::profiler_stop
-[debug/optimizer] removing declaration for unused function spicy_rt::atEod
-[debug/optimizer] removing declaration for unused function spicy_rt::backtrack
-[debug/optimizer] removing declaration for unused function spicy_rt::confirm
-[debug/optimizer] removing declaration for unused function spicy_rt::createContext
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_connect
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_disconnect
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_forward
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_forward_eod
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_init
-[debug/optimizer] removing declaration for unused function spicy_rt::printParserState
-[debug/optimizer] removing declaration for unused function spicy_rt::reject
-[debug/optimizer] removing declaration for unused function spicy_rt::setContext
-[debug/optimizer] removing declaration for unused function spicy_rt::unit_find
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForEod
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInput
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInput
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInputOrEod
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInputOrEod
 [debug/optimizer] removing field for unused method foo::A::__on_0x25_confirmed
 [debug/optimizer] removing field for unused method foo::A::__on_0x25_done
 [debug/optimizer] removing field for unused method foo::A::__on_0x25_error

diff --git a/tests/Baseline/spicy.optimization.unused-functions/opt.hlt b/tests/Baseline/spicy.optimization.unused-functions/opt.hlt
@@ -60,6 +60,9 @@ const bool __feat%foo@@F%supports_filters = False;
 const bool __feat%foo@@F%supports_sinks = False;
 const bool __feat%foo@@F%synchronization = False;
 
+public function void f2() {
+}
+
 init function void __register_foo_A() {
 }
 

diff --git a/tests/Baseline/spicy.optimization.unused-types/log b/tests/Baseline/spicy.optimization.unused-types/log
@@ -404,33 +404,6 @@
 [debug/optimizer] removing declaration for unused function foo::Priv6::parse1
 [debug/optimizer] removing declaration for unused function foo::Priv6::parse2
 [debug/optimizer] removing declaration for unused function foo::Priv6::parse3
-[debug/optimizer] removing declaration for unused function hilti::abort
-[debug/optimizer] removing declaration for unused function hilti::current_time
-[debug/optimizer] removing declaration for unused function hilti::debug
-[debug/optimizer] removing declaration for unused function hilti::exception_what
-[debug/optimizer] removing declaration for unused function hilti::mktime
-[debug/optimizer] removing declaration for unused function hilti::print
-[debug/optimizer] removing declaration for unused function hilti::printValues
-[debug/optimizer] removing declaration for unused function hilti::profiler_start
-[debug/optimizer] removing declaration for unused function hilti::profiler_stop
-[debug/optimizer] removing declaration for unused function spicy_rt::atEod
-[debug/optimizer] removing declaration for unused function spicy_rt::backtrack
-[debug/optimizer] removing declaration for unused function spicy_rt::confirm
-[debug/optimizer] removing declaration for unused function spicy_rt::createContext
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_connect
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_disconnect
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_forward
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_forward_eod
-[debug/optimizer] removing declaration for unused function spicy_rt::filter_init
-[debug/optimizer] removing declaration for unused function spicy_rt::printParserState
-[debug/optimizer] removing declaration for unused function spicy_rt::reject
-[debug/optimizer] removing declaration for unused function spicy_rt::setContext
-[debug/optimizer] removing declaration for unused function spicy_rt::unit_find
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForEod
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInput
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInput
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInputOrEod
-[debug/optimizer] removing declaration for unused function spicy_rt::waitForInputOrEod
 [debug/optimizer] removing field for unused method foo::Priv10::__on_0x25_confirmed
 [debug/optimizer] removing field for unused method foo::Priv10::__on_0x25_done
 [debug/optimizer] removing field for unused method foo::Priv10::__on_0x25_error

diff --git a/tests/hilti/optimization/unused_function.hlt b/tests/hilti/optimization/unused_function.hlt
@@ -18,7 +18,7 @@ function bool private_unused1() { return private_unused(); }
 function bool private_used() { return False; }
 global x = private_used();
 
-# Public unused functions should get optimized out.
+# Public unused functions should not get optimized out.
 public function bool public_unused() { return False; }
 
 # Extern functions should not get optimized out, regardless whether they are public or not.