Merge remote-tracking branch 'origin/topic/jsiwek/brofiler'

* origin/topic/jsiwek/brofiler:
  Fix superfluous/duplicate data getting in to testing coverage log.
  Add "# @no-test" tag to blacklist statements from test coverage analysis.
  Test coverage integration for external tests and complete suite.
  Integrate Bro script coverage profiling with the btest suite.
  Add simple profiling class to accumulate Stmt usage stats across runs.

Renaming environment variable BROFILER_FILE to BRO_PROFILER_FILE for
consistency. Yeah, I know, such a nice name! :)

aux/btest

@@ -1 +1 @@
-Subproject commit 5350e4652b44ce1fbd9fffe1228d097fb04247cd
+Subproject commit ee87db37b520b88a55323a9767234c30b801e439

src/Brofiler.cc

@@ -0,0 +1,84 @@
+#include <cstdio>
+#include <utility>
+#include <algorithm>
+#include "Brofiler.h"
+#include "util.h"
+
+Brofiler::Brofiler()
+	: delim('\t'), ignoring(0)
+	{
+	}
+
+Brofiler::~Brofiler()
+	{
+	}
+
+bool Brofiler::ReadStats()
+	{
+	char* bf = getenv("BRO_PROFILER_FILE");
+	if ( ! bf )
+		return false;
+
+	FILE* f = fopen(bf, "r");
+	if ( ! f )
+		return false;
+
+	char line[16384];
+	string delimiter;
+	delimiter = delim;
+
+	while( fgets(line, sizeof(line), f) )
+		{
+		line[strlen(line) - 1] = 0; //remove newline
+		string cnt(strtok(line, delimiter.c_str()));
+		string location(strtok(0, delimiter.c_str()));
+		string desc(strtok(0, delimiter.c_str()));
+		pair<string, string> location_desc(location, desc);
+		uint64 count;
+		atoi_n(cnt.size(), cnt.c_str(), 0, 10, count);
+		usage_map[location_desc] = count;
+		}
+
+	fclose(f);
+	return true;
+	}
+
+bool Brofiler::WriteStats()
+	{
+	char* bf = getenv("BRO_PROFILER_FILE");
+	if ( ! bf ) return false;
+
+	FILE* f = fopen(bf, "w");
+	if ( ! f )
+		{
+		reporter->Error("Failed to open BRO_PROFILER_FILE destination '%s' for writing\n", bf);
+		return false;
+		}
+
+	for ( list<const Stmt*>::const_iterator it = stmts.begin();
+	      it != stmts.end(); ++it )
+		{
+		ODesc location_info;
+		(*it)->GetLocationInfo()->Describe(&location_info);
+		ODesc desc_info;
+		(*it)->Describe(&desc_info);
+		string desc(desc_info.Description());
+		for_each(desc.begin(), desc.end(), canonicalize_desc());
+		pair<string, string> location_desc(location_info.Description(), desc);
+		if ( usage_map.find(location_desc) != usage_map.end() )
+			usage_map[location_desc] += (*it)->GetAccessCount();
+		else
+			usage_map[location_desc] = (*it)->GetAccessCount();
+		}
+
+	map<pair<string, string>, uint64 >::const_iterator it;
+	for ( it = usage_map.begin(); it != usage_map.end(); ++it )
+		{
+		fprintf(f, "%"PRIu64"%c%s%c%s\n", it->second, delim,
+				it->first.first.c_str(), delim, it->first.second.c_str());
+		}
+
+	fclose(f);
+	return true;
+	}
+

src/Brofiler.h

@@ -0,0 +1,79 @@
+#ifndef BROFILER_H_
+#define BROFILER_H_
+
+#include <map>
+#include <utility>
+#include <list>
+#include <Stmt.h>
+
+
+/**
+ * A simple class for managing stats of Bro script coverage across Bro runs.
+ */
+class Brofiler {
+public:
+	Brofiler();
+	virtual ~Brofiler();
+
+	/**
+	 * Imports Bro script Stmt usage information from file pointed to by
+	 * environment variable BRO_PROFILER_FILE.
+	 *
+	 * @return: true if usage info was read, otherwise false.
+	 */
+	bool ReadStats();
+
+	/**
+	 * Combines usage stats from current run with any read from ReadStats(),
+	 * then writes information to file pointed to by environment variable
+	 * BRO_PROFILER_FILE.
+	 *
+	 * @return: true when usage info is written, otherwise false.
+	 */
+	bool WriteStats();
+
+	void SetDelim(char d) { delim = d; }
+
+	void IncIgnoreDepth() { ignoring++; }
+	void DecIgnoreDepth() { ignoring--; }
+
+	void AddStmt(const Stmt* s) { if ( ignoring == 0 ) stmts.push_back(s); }
+
+private:
+	/**
+	 * The current, global Brofiler instance creates this list at parse-time.
+	 */
+	list<const Stmt*> stmts;
+
+	/**
+	 * Indicates whether new statments will not be considered as part of
+	 * coverage statistics because it was marked with the @no-test tag.
+	 */
+	unsigned int ignoring;
+
+	/**
+	 * This maps Stmt location-desc pairs to the total number of times that
+	 * Stmt has been executed.  The map can be initialized from a file at
+	 * startup time and modified at shutdown time before writing back
+	 * to a file.
+	 */
+	map<pair<string, string>, uint64> usage_map;
+
+	/**
+	 * The character to use to delimit Brofiler output files.  Default is '\t'.
+	 */
+	char delim;
+
+	/**
+	 * A canonicalization routine for Stmt descriptions containing characters
+	 * that don't agree with the output format of Brofiler.
+	 */
+	struct canonicalize_desc {
+		void operator() (char& c)
+			{
+			if ( c == '\n' ) c = ' ';
+			}
+	};
+};
+
+#endif /* BROFILER_H_ */

src/CMakeLists.txt

@@ -282,6 +282,7 @@ set(bro_SRCS
     BPF_Program.cc
     BroDoc.cc
     BroDocObj.cc
+    Brofiler.cc
     BroString.cc
     CCL.cc
     ChunkedIO.cc

src/Stmt.cc

@@ -258,6 +258,8 @@ static BroFile* print_stdout = 0;
 
 Val* PrintStmt::DoExec(val_list* vals, stmt_flow_type& /* flow */) const
 	{
+	RegisterAccess();
+
 	if ( ! print_stdout )
 		print_stdout = new BroFile(stdout);
 

src/Stmt.h

@@ -52,6 +52,7 @@ public:
 
 	void RegisterAccess() const	{ last_access = network_time; access_count++; }
 	void AccessStats(ODesc* d) const;
+	uint32 GetAccessCount() const { return access_count; }
 
 	virtual void Describe(ODesc* d) const;
 

src/main.cc

@@ -47,10 +47,13 @@ extern "C" void OPENSSL_add_all_algorithms_conf(void);
 #include "ConnCompressor.h"
 #include "DPM.h"
 #include "BroDoc.h"
+#include "Brofiler.h"
 #include "LogWriterAscii.h"
 
 #include "binpac_bro.h"
 
+Brofiler brofiler;
+
 #ifndef HAVE_STRSEP
 extern "C" {
 char* strsep(char**, const char*);
@@ -195,6 +198,7 @@ void usage()
 	fprintf(stderr, "    $BRO_DNS_FAKE                  | disable DNS lookups (%s)\n", bro_dns_fake());
 	fprintf(stderr, "    $BRO_SEED_FILE                 | file to load seeds from (not set)\n");
 	fprintf(stderr, "    $BRO_LOG_SUFFIX                | ASCII log file extension (.%s)\n", LogWriterAscii::LogExt().c_str());
+	fprintf(stderr, "    $BRO_PROFILER_FILE             | Output file for script execution statistics (not set)\n");
 
 	exit(1);
 	}
@@ -261,6 +265,8 @@ void terminate_bro()
 
 	terminating = true;
 
+	brofiler.WriteStats();
+
 	EventHandlerPtr bro_done = internal_handler("bro_done");
 	if ( bro_done )
 		mgr.QueueEvent(bro_done, new val_list);
@@ -336,6 +342,8 @@ static void bro_new_handler()
 
 int main(int argc, char** argv)
 	{
+	brofiler.ReadStats();
+
 	bro_argc = argc;
 	bro_argv = new char* [argc];
 

src/parse.y

@@ -29,6 +29,8 @@
 
 %token TOK_DOC TOK_POST_DOC
 
+%token TOK_NO_TEST
+
 %left ',' '|'
 %right '=' TOK_ADD_TO TOK_REMOVE_FROM
 %right '?' ':' TOK_USING
@@ -42,6 +44,7 @@
 %right '!'
 %left '$' '[' ']' '(' ')' TOK_HAS_FIELD TOK_HAS_ATTR
 
+%type <b> opt_no_test opt_no_test_block
 %type <str> TOK_ID TOK_PATTERN_TEXT single_pattern TOK_DOC TOK_POST_DOC
 %type <str_l> opt_doc_list opt_post_doc_list
 %type <id> local_id global_id def_global_id event_id global_or_event_id resolve_id begin_func
@@ -80,10 +83,12 @@
 #include "Reporter.h"
 #include "BroDoc.h"
 #include "BroDocObj.h"
+#include "Brofiler.h"
 
 #include <list>
 #include <string>
 
+extern Brofiler brofiler;
 extern BroDoc* current_reST_doc;
 extern int generate_documentation;
 extern std::list<std::string>* reST_doc_comments;
@@ -194,6 +199,7 @@ static std::list<std::string>* concat_opt_docs (std::list<std::string>* pre,
 %}
 
 %union {
+	bool b;
 	char* str;
 	std::list<std::string>* str_l;
 	ID* id;
@@ -1313,22 +1319,28 @@ attr:
 	;
 
 stmt:
-		'{' stmt_list '}'
+		'{' opt_no_test_block stmt_list '}'
 			{
-			set_location(@1, @3);
+			set_location(@1, @4);
-			$$ = $2;
+			$$ = $3;
+			if ( $2 )
+			    brofiler.DecIgnoreDepth();
 			}
 
-	|	TOK_PRINT expr_list ';'
+	|	TOK_PRINT expr_list ';' opt_no_test
 			{
 			set_location(@1, @3);
 			$$ = new PrintStmt($2);
+			if ( ! $4 )
+			    brofiler.AddStmt($$);
 			}
 
-	|	TOK_EVENT event ';'
+	|	TOK_EVENT event ';' opt_no_test
 			{
 			set_location(@1, @3);
 			$$ = new EventStmt($2);
+			if ( ! $4 )
+			    brofiler.AddStmt($$);
 			}
 
 	|	TOK_IF '(' expr ')' stmt
@@ -1350,54 +1362,72 @@ stmt:
 			}
 
 	|	for_head stmt
-			{ $1->AsForStmt()->AddBody($2); }
+			{
+			$1->AsForStmt()->AddBody($2);
+			}
 
-	|	TOK_NEXT ';'
+	|	TOK_NEXT ';' opt_no_test
 			{
 			set_location(@1, @2);
 			$$ = new NextStmt;
+			if ( ! $3 )
+			    brofiler.AddStmt($$);
 			}
 
-	|	TOK_BREAK ';'
+	|	TOK_BREAK ';' opt_no_test
 			{
 			set_location(@1, @2);
 			$$ = new BreakStmt;
+			if ( ! $3 )
+			    brofiler.AddStmt($$);
 			}
 
-	|	TOK_RETURN ';'
+	|	TOK_RETURN ';' opt_no_test
 			{
 			set_location(@1, @2);
 			$$ = new ReturnStmt(0);
+			if ( ! $3 )
+			    brofiler.AddStmt($$);
 			}
 
-	|	TOK_RETURN expr ';'
+	|	TOK_RETURN expr ';' opt_no_test
 			{
 			set_location(@1, @2);
 			$$ = new ReturnStmt($2);
+			if ( ! $4 )
+			    brofiler.AddStmt($$);
 			}
 
-	|	TOK_ADD expr ';'
+	|	TOK_ADD expr ';' opt_no_test
 			{
 			set_location(@1, @3);
 			$$ = new AddStmt($2);
+			if ( ! $4 )
+			    brofiler.AddStmt($$);
 			}
 
-	|	TOK_DELETE expr ';'
+	|	TOK_DELETE expr ';' opt_no_test
 			{
 			set_location(@1, @3);
 			$$ = new DelStmt($2);
+			if ( ! $4 )
+			    brofiler.AddStmt($$);
 			}
 
-	|	TOK_LOCAL local_id opt_type init_class opt_init opt_attr ';'
+	|	TOK_LOCAL local_id opt_type init_class opt_init opt_attr ';' opt_no_test
 			{
 			set_location(@1, @7);
 			$$ = add_local($2, $3, $4, $5, $6, VAR_REGULAR);
+			if ( ! $8 )
+			    brofiler.AddStmt($$);
 			}
 
-	|	TOK_CONST local_id opt_type init_class opt_init opt_attr ';'
+	|	TOK_CONST local_id opt_type init_class opt_init opt_attr ';' opt_no_test
 			{
 			set_location(@1, @6);
 			$$ = add_local($2, $3, $4, $5, $6, VAR_CONST);
+			if ( ! $8 )
+			    brofiler.AddStmt($$);
 			}
 
 	|	TOK_WHEN '(' expr ')' stmt
@@ -1406,10 +1436,12 @@ stmt:
 			$$ = new WhenStmt($3, $5, 0, 0, false);
 			}
 
-	|	TOK_WHEN '(' expr ')' stmt TOK_TIMEOUT expr '{' stmt_list '}'
+	|	TOK_WHEN '(' expr ')' stmt TOK_TIMEOUT expr '{' opt_no_test_block stmt_list '}'
 			{
-			set_location(@3, @8);
+			set_location(@3, @9);
-			$$ = new WhenStmt($3, $5, $9, $7, false);
+			$$ = new WhenStmt($3, $5, $10, $7, false);
+			if ( $9 )
+			    brofiler.DecIgnoreDepth();
 			}
 
 
@@ -1419,16 +1451,20 @@ stmt:
 			$$ = new WhenStmt($4, $6, 0, 0, true);
 			}
 
-	|	TOK_RETURN TOK_WHEN '(' expr ')' stmt TOK_TIMEOUT expr '{' stmt_list '}'
+	|	TOK_RETURN TOK_WHEN '(' expr ')' stmt TOK_TIMEOUT expr '{' opt_no_test_block stmt_list '}'
 			{
-			set_location(@4, @9);
+			set_location(@4, @10);
-			$$ = new WhenStmt($4, $6, $10, $8, true);
+			$$ = new WhenStmt($4, $6, $11, $8, true);
+			if ( $10 )
+			    brofiler.DecIgnoreDepth();
 			}
 
-	|	expr ';'
+	|	expr ';' opt_no_test
 			{
 			set_location(@1, @2);
 			$$ = new ExprStmt($1);
+			if ( ! $3 )
+			    brofiler.AddStmt($$);
 			}
 
 	|	';'
@@ -1626,6 +1662,18 @@ opt_doc_list:
 			{ $$ = 0; }
 	;
 
+opt_no_test:
+		TOK_NO_TEST
+			{ $$ = true; }
+	|
+			{ $$ = false; }
+
+opt_no_test_block:
+		TOK_NO_TEST
+			{ $$ = true; brofiler.IncIgnoreDepth(); }
+	|
+			{ $$ = false; }
+
 %%
 
 int yyerror(const char msg[])

src/scan.l

@@ -216,6 +216,8 @@ ESCSEQ	(\\([^\n]|[0-7]+|x[[:xdigit:]]+))
 		}
 }
 
+#{OWS}@no-test.* return TOK_NO_TEST;
+
 #.*	/* eat comments */
 
 {WS}	/* eat whitespace */

src/util.cc

@@ -376,6 +376,7 @@ template<class T> int atoi_n(int len, const char* s, const char** end, int base,
 // Instantiate the ones we need.
 template int atoi_n<int>(int len, const char* s, const char** end, int base, int& result);
 template int atoi_n<int64_t>(int len, const char* s, const char** end, int base, int64_t& result);
+template int atoi_n<uint64_t>(int len, const char* s, const char** end, int base, uint64_t& result);
 
 char* uitoa_n(uint64 value, char* str, int n, int base, const char* prefix)
 	{

testing/.gitignore

@@ -0,0 +1 @@
+coverage.log

testing/Makefile

@@ -3,6 +3,11 @@ DIRS=btest external
 
 all:
 	@for repo in $(DIRS); do (cd $$repo && make ); done
+	@cp btest/coverage.log `mktemp brocov.tmp.XXX`
+	@for f in external/*/coverage.log; do cp $$f `mktemp brocov.tmp.XXX`; done
+	@echo "Complete test suite code coverage:"
+	@./scripts/coverage-calc "brocov.tmp.*" coverage.log `pwd`/../scripts
+	@rm -f brocov.tmp.*
 
 brief:
 	@for repo in $(DIRS); do (cd $$repo && make brief ); done

testing/btest/.gitignore

@@ -1,2 +1,3 @@
 .tmp
 diag.log
+coverage.log

testing/btest/Baseline/coverage.coverage-blacklist/output

@@ -0,0 +1,5 @@
+1	/Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/coverage.coverage-blacklist/coverage-blacklist.bro, line 13	print cover me;
+1	/Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/coverage.coverage-blacklist/coverage-blacklist.bro, line 17	print always executed;
+0	/Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/coverage.coverage-blacklist/coverage-blacklist.bro, line 26	print also impossible, but included in code coverage analysis;
+1	/Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/coverage.coverage-blacklist/coverage-blacklist.bro, line 29	print success;
+1	/Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/coverage.coverage-blacklist/coverage-blacklist.bro, line 5	print first;

testing/btest/Makefile

@@ -5,7 +5,9 @@ BTEST=../../aux/btest/btest
 all:
     # Showing all tests.
 	@rm -f $(DIAG)
+	@rm -f .tmp/script-coverage*
 	@$(BTEST) -f $(DIAG)
+	@../scripts/coverage-calc ".tmp/script-coverage*" coverage.log `pwd`/../../scripts
 
 brief:
     # Brief output showing only failed tests.

testing/btest/btest.cfg

@@ -10,9 +10,12 @@ BROPATH=`bash -c %(testbase)s/../../build/bro-path-dev`
 BRO_SEED_FILE=%(testbase)s/random.seed
 TZ=UTC
 LC_ALL=C
-PATH=%(testbase)s/../../build/src:%(testbase)s/../../aux/btest:%(default_path)s
+BTEST_PATH=%(testbase)s/../../aux/btest
+PATH=%(testbase)s/../../build/src:%(testbase)s/../scripts:%(testbase)s/../../aux/btest:%(default_path)s
 TRACES=%(testbase)s/Traces
 SCRIPTS=%(testbase)s/../scripts
 DIST=%(testbase)s/../..
 BUILD=%(testbase)s/../../build
 TEST_DIFF_CANONIFIER=$SCRIPTS/diff-canonifier
+TMPDIR=%(testbase)s/.tmp
+BROFILER_FILE=%(testbase)s/.tmp/script-coverage

testing/btest/coverage/coverage-blacklist.bro

@@ -0,0 +1,29 @@
+# @TEST-EXEC: BROFILER_FILE=coverage bro -b %INPUT
+# @TEST-EXEC: grep %INPUT coverage | sort -k2 >output
+# @TEST-EXEC: btest-diff output
+
+print "first";
+
+if ( F )
+	{ # @no-test
+	print "hello";
+	print "world";
+	}
+
+print "cover me";
+
+if ( T )
+	{
+	print "always executed";
+	}
+
+print "don't cover me"; # @no-test
+
+if ( 0 + 0 == 1 ) print "impossible"; # @no-test
+
+if ( 1 == 0 )
+	{
+	print "also impossible, but included in code coverage analysis";
+	}
+
+print "success";

testing/external/subdir-btest.cfg

@@ -17,3 +17,4 @@ TRACES=%(testbase)s/Traces
 SCRIPTS=%(testbase)s/../scripts
 DIST=%(testbase)s/../../..
 BUILD=%(testbase)s/../../../build
+BROFILER_FILE=%(testbase)s/.tmp/script-coverage

testing/scripts/btest-bg-run

@@ -0,0 +1,7 @@
+#! /usr/bin/env bash
+
+# This is a wrapper script to btest's real btest-bg-run.  It's used
+# when collecting Bro script coverage statistics so that two independent
+# Bro processing don't try to write those usage statistics to the same file.
+
+BROFILER_FILE=`mktemp -t script-coverage` $BTEST_PATH/btest-bg-run $@

testing/scripts/coverage-calc

@@ -0,0 +1,51 @@
+#! /usr/bin/env python
+
+# This script aggregates many files containing Bro script coverage information
+# into a single file and reports the overall coverage information. Usage:
+#
+#   coverage-calc <quoted glob of filenames> <output file> <script dir>
+#
+# The last argument is used to point to a root directory containing all
+# the Bro distribution's scripts.  It's used to cull out test scripts
+# that are not part of the distribution and which should not count towrads
+# the coverage calculation.
+
+import os
+import sys
+import glob
+
+stats = {}
+inputglob = sys.argv[1]
+outputfile = sys.argv[2]
+scriptdir = os.path.abspath(sys.argv[3])
+
+for filename in glob.glob(inputglob):
+    with open(filename, 'r') as f:
+        for line in f.read().splitlines():
+            parts = line.split("\t")
+            exec_count = int(parts[0])
+            location = os.path.normpath(parts[1])
+            # ignore scripts that don't appear to be part of Bro distribution
+            if not location.startswith(scriptdir):
+                continue
+            desc = parts[2]
+            # keying by location + desc may result in duplicate data
+            # as some descs change as a result of differing configurations
+            # producing record (re)definitions
+            key = location
+            if key in stats:
+                stats[key][0] += exec_count
+            else:
+                stats[key] = [exec_count, location, desc]
+
+with open(outputfile, 'w') as f:
+    for k in sorted(stats, key=lambda i: stats[i][1]):
+        f.write("%s\t%s\t%s\n" % (stats[k][0], stats[k][1], stats[k][2]))
+
+num_covered = 0
+for k in stats:
+    if stats[k][0] > 0:
+        num_covered += 1
+
+if len(stats) > 0:
+    print "%s/%s (%.1f%%) Bro script statements covered." % (num_covered, len(stats), float(num_covered)/len(stats)*100)