From 20294d372cfb37ae78432d2a52a4e268f1ed1cf2 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 10 Jul 2020 01:53:26 -0700 Subject: [PATCH] Fix wrong frame offsets for locals of alternate event/hook prototypes Local frame offsets were being assigned based on number of the alternate prototype's parameters, which may end up having less total parameters than the canonical prototype, causing the local value to incorrectly overwrite an event/hook argument value. --- src/Type.h | 2 + src/Var.cc | 50 +++++++++++++++---- .../hidden-error | 3 ++ .../out | 9 ++-- .../alternate-prototypes-deprecated-args.zeek | 21 +++++++- 5 files changed, 71 insertions(+), 14 deletions(-) create mode 100644 testing/btest/Baseline/language.alternate-prototypes-deprecated-args/hidden-error diff --git a/src/Type.h b/src/Type.h index a2054473d3..a28a32c45d 100644 --- a/src/Type.h +++ b/src/Type.h @@ -429,6 +429,8 @@ public: bool deprecated; std::string deprecation_msg; RecordTypePtr args; + // Maps from parameter index in canonical prototype to + // parameter index in this alternate prorotype. std::map offsets; }; diff --git a/src/Var.cc b/src/Var.cc index 535c9ab096..16e474ea02 100644 --- a/src/Var.cc +++ b/src/Var.cc @@ -96,7 +96,7 @@ static bool add_prototype(const zeek::detail::IDPtr& id, zeek::Type* t, return false; } - offsets[i] = o; + offsets[o] = i; } auto deprecated = false; @@ -604,24 +604,54 @@ void begin_func(zeek::detail::IDPtr id, const char* module_name, else id->SetType(t); + const auto& args = t->Params(); + const auto& canon_args = id->GetType()->AsFuncType()->Params(); + zeek::detail::push_scope(std::move(id), std::move(attrs)); - const auto& args = t->Params(); - int num_args = args->NumFields(); - - for ( int i = 0; i < num_args; ++i ) + for ( int i = 0; i < canon_args->NumFields(); ++i ) { - zeek::TypeDecl* arg_i = args->FieldDecl(i); + zeek::TypeDecl* arg_i; + bool hide = false; + + if ( prototype ) + { + auto it = prototype->offsets.find(i); + + if ( it == prototype->offsets.end() ) + { + // Alternate prototype hides this param + hide = true; + arg_i = canon_args->FieldDecl(i); + } + else + { + // Alternate prototype maps this param to another index + arg_i = args->FieldDecl(it->second); + } + } + else + { + if ( i < args->NumFields() ) + arg_i = args->FieldDecl(i); + else + break; + } + auto arg_id = zeek::detail::lookup_ID(arg_i->id, module_name); if ( arg_id && ! arg_id->IsGlobal() ) arg_id->Error("argument name used twice"); - arg_id = zeek::detail::install_ID(arg_i->id, module_name, false, false); - arg_id->SetType(arg_i->type); + const char* local_name = arg_i->id; - if ( prototype ) - arg_id->SetOffset(prototype->offsets[i]); + if ( hide ) + // Note the illegal '-' in hidden name implies we haven't + // clobbered any local variable names. + local_name = fmt("%s-hidden", local_name); + + arg_id = zeek::detail::install_ID(local_name, module_name, false, false); + arg_id->SetType(arg_i->type); } if ( zeek::detail::Attr* depr_attr = find_attr(zeek::detail::current_scope()->Attrs().get(), diff --git a/testing/btest/Baseline/language.alternate-prototypes-deprecated-args/hidden-error b/testing/btest/Baseline/language.alternate-prototypes-deprecated-args/hidden-error new file mode 100644 index 0000000000..3e7b9120c4 --- /dev/null +++ b/testing/btest/Baseline/language.alternate-prototypes-deprecated-args/hidden-error @@ -0,0 +1,3 @@ +warning in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.alternate-prototypes-deprecated-args/alternate-prototypes-deprecated-args.zeek, line 11 and /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.alternate-prototypes-deprecated-args/alternate-prototypes-deprecated-args.zeek, line 7: use of deprecated parameter 'b': Don't use 'b' (event(a:string; b:string; c:string;)) +warning in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.alternate-prototypes-deprecated-args/alternate-prototypes-deprecated-args.zeek, line 30 and /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.alternate-prototypes-deprecated-args/alternate-prototypes-deprecated-args.zeek, line 9: use of deprecated 'myev' prototype: Don't use this prototype (event(a:string; b:string;)) +error in ./hide.zeek, line 5: unknown identifier b, at or near "b" diff --git a/testing/btest/Baseline/language.alternate-prototypes-deprecated-args/out b/testing/btest/Baseline/language.alternate-prototypes-deprecated-args/out index 8a8d8e5b88..b3686a0b23 100644 --- a/testing/btest/Baseline/language.alternate-prototypes-deprecated-args/out +++ b/testing/btest/Baseline/language.alternate-prototypes-deprecated-args/out @@ -1,5 +1,8 @@ -warning in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.alternate-prototypes-deprecated-args/alternate-prototypes-deprecated-args.zeek, line 9 and /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.alternate-prototypes-deprecated-args/alternate-prototypes-deprecated-args.zeek, line 5: use of deprecated parameter 'b': Don't use 'b' (event(a:string; b:string; c:string;)) -warning in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.alternate-prototypes-deprecated-args/alternate-prototypes-deprecated-args.zeek, line 19 and /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.alternate-prototypes-deprecated-args/alternate-prototypes-deprecated-args.zeek, line 7: use of deprecated 'myev' prototype: Don't use this prototype (event(a:string; b:string;)) +warning in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.alternate-prototypes-deprecated-args/alternate-prototypes-deprecated-args.zeek, line 11 and /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.alternate-prototypes-deprecated-args/alternate-prototypes-deprecated-args.zeek, line 7: use of deprecated parameter 'b': Don't use 'b' (event(a:string; b:string; c:string;)) +warning in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.alternate-prototypes-deprecated-args/alternate-prototypes-deprecated-args.zeek, line 30 and /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.alternate-prototypes-deprecated-args/alternate-prototypes-deprecated-args.zeek, line 9: use of deprecated 'myev' prototype: Don't use this prototype (event(a:string; b:string;)) myev (canon), one, two, three -myev (new), one, three +myev (new), one, three, [1, 2, 3] +myev (new), one, three, 0 +myev (new), one, three, 1 +myev (new), one, three, 2 myev (old), one, two diff --git a/testing/btest/language/alternate-prototypes-deprecated-args.zeek b/testing/btest/language/alternate-prototypes-deprecated-args.zeek index 659c4c7a16..d6c40425eb 100644 --- a/testing/btest/language/alternate-prototypes-deprecated-args.zeek +++ b/testing/btest/language/alternate-prototypes-deprecated-args.zeek @@ -1,6 +1,8 @@ # @TEST-EXEC: zeek -b %INPUT >out 2>&1 # +# @TEST-EXEC-FAIL: zeek -b %INPUT hide.zeek >hidden-error 2>&1 # @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff hidden-error global myev: event(a: string, b: string &deprecated="Don't use 'b'", c: string); global myev: event(a: string, c: string); @@ -13,7 +15,16 @@ event myev(a: string, b: string, c: string) &priority=11 event myev(a: string, c: string) &priority = 7 { - print "myev (new)", a, c; + local ddd = vector(1,2,3); + print "myev (new)", a, c, ddd; + } + +global eee = vector(1,2,3); + +event myev(a: string, c: string) &priority = 6 + { + for ( o in eee ) + print "myev (new)", a, c, o; } event myev(a: string, b: string) &priority = 5 @@ -26,3 +37,11 @@ event zeek_init() event myev("one", "two", "three"); } +@TEST-START-FILE hide.zeek +event myev(a: string, c: string) &priority = 7 + { + local ddd = vector(1,2,3); + print "myev (new)", a, c, ddd; + print b; + } +@TEST-END-FILE