Merge remote-tracking branch 'origin/topic/jsiwek/32bit-compat'

* origin/topic/jsiwek/32bit-compat:
  Improve formatting of doubles that are close to integers
  Improve HTTP version number comparisons
  Add a 32-bit task to Cirrus CI config
  Replace va_list fmt() overload with vfmt()
  Format tables indexed by patterns consistently across 32-bit/64-bit
  Format interval values consistently across 32-bit/64-bit platforms

.cirrus.yml

@@ -77,6 +77,13 @@ debian9_task:
     << : *RESOURCES_TEMPLATE
   << : *CI_TEMPLATE
 
+debian9_32bit_task:
+  container:
+    # Debian 9 EOL: June 2022
+    dockerfile: ci/debian-9-32bit/Dockerfile
+    << : *RESOURCES_TEMPLATE
+  << : *CI_TEMPLATE
+
 ubuntu18_task:
   container:
     # Ubuntu 18.04 EOL: April 2023

CHANGES

@@ -1,4 +1,44 @@
 
+3.2.0-dev.115 | 2020-02-24 18:48:50 -0700
+
+  * Improve formatting of doubles that are close to integers
+
+    Now checks for approximate floating point equality so that more doubles
+    get properly disambiguated from integers (Jon Siwek, Corelight)
+
+  * Improve HTTP version number comparisons
+
+    Previous use of floating point comparisons was not always stable. (Jon Siwek, Corelight)
+
+  * Add a 32-bit task to Cirrus CI config (Jon Siwek, Corelight)
+
+  * Replace va_list fmt() overload with vfmt()
+
+    Using an overload that takes a va_list argument potentially causes
+    accidental misuse on platforms (e.g. 32-bit) where va_list is
+    implemented as a type that may collide with commonly-used argument
+    types.
+
+    For example:
+
+        char* c = copy_string("hi");
+        fmt("%s", (const char*)c);
+        fmt("%s", c);
+
+    The first fmt() call correctly goes through fmt(const char*, ...) first,
+    but the second mistakenly goes through fmt(const char*, va_list) first
+    because variadic function overloads have lower priority during overload
+    resolution and va_list on a 32-bit system happens to be defined as a
+    pointer type that can match with "char*" but not "const char*". (Jon Siwek, Corelight)
+
+  * Format tables indexed by patterns consistently across 32-bit/64-bit
+
+    Uses a full 64 bit integer for length values regardless of actual size_t
+    to get consistent results between either 32-bit and 64-bit platforms. (Jon Siwek, Corelight)
+
+  * Format interval values consistently across 32-bit/64-bit platforms (Jon Siwek, Corelight)
+
+
 3.2.0-dev.108 | 2020-02-24 17:24:07 -0800
 
   * Change OpaqueVal/HashVal APIs to use IntrusivePtr (Max Kellermann)

NEWS

@@ -20,6 +20,12 @@ Changed Functionality
 Removed Functionality
 ---------------------
 
+- The fmt() function which takes a va_list argument is replaced, use
+  the new vfmt() function for equivalent functionality.  The former is
+  deprecated because overloading it with the variadic fmt() function
+  can cause the unintended overload to be chosen depending on how the
+  platform implements va_list.
+
 Deprecated Functionality
 ------------------------
 

VERSION

@@ -1 +1 @@
-3.2.0-dev.108
+3.2.0-dev.115

ci/debian-9-32bit/Dockerfile

@@ -0,0 +1,40 @@
+FROM i386/debian:9
+
+RUN apt-get update && apt-get -y install \
+    git \
+    cmake \
+    make \
+    gcc \
+    g++ \
+    flex \
+    bison \
+    libpcap-dev \
+    libssl-dev \
+    python3 \
+    python3-dev \
+    python3-pip\
+    swig \
+    zlib1g-dev \
+    libkrb5-dev \
+    bsdmainutils \
+    sqlite3 \
+    curl \
+    wget \
+    xz-utils \
+    clang-7 \
+    libc++-7-dev \
+    libc++abi-7-dev \
+  && rm -rf /var/lib/apt/lists/*
+
+RUN update-alternatives --install /usr/bin/cc cc /usr/bin/clang-7 100
+RUN update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++-7 100
+
+# Many distros adhere to PEP 394's recommendation for `python` = `python2` so
+# this is a simple workaround until we drop Python 2 support and explicitly
+# use `python3` for all invocations (e.g. in shebangs).
+RUN ln -sf /usr/bin/python3 /usr/local/bin/python
+RUN ln -sf /usr/bin/pip3 /usr/local/bin/pip
+
+RUN pip install junit2html
+
+ENV CXXFLAGS=-stdlib=libc++

src/CompHash.cc

@@ -154,10 +154,10 @@ char* CompositeHash::SingleValHash(int type_check, char* kp0,
 				v->AsPattern()->AnywherePatternText()
 			};
 
-			size_t* kp;
+			uint64_t* kp;
 			for ( int i = 0; i < 2; i++ )
 				{
-				kp = AlignAndPadType<size_t>(kp0+i);
+				kp = AlignAndPadType<uint64_t>(kp0+i);
 				*kp = strlen(texts[i]) + 1;
 				}
 
@@ -505,7 +505,7 @@ int CompositeHash::SingleTypeKeySize(BroType* bt, const Val* v,
 			if ( ! v )
 				return (optional && ! calc_static_size) ? sz : 0;
 
-			sz = SizeAlign(sz, 2 * sizeof(size_t));
+			sz = SizeAlign(sz, 2 * sizeof(uint64_t));
 			sz += strlen(v->AsPattern()->PatternText())
 				+ strlen(v->AsPattern()->AnywherePatternText()) + 2; // 2 for null terminators
 			break;
@@ -892,7 +892,7 @@ const char* CompositeHash::RecoverOneVal(const HashKey* k, const char* kp0,
 				}
 			else
 				{
-				const size_t* const len = AlignType<size_t>(kp0);
+				const uint64_t* const len = AlignType<uint64_t>(kp0);
 
 				kp1 = reinterpret_cast<const char*>(len+2);
 				re = new RE_Matcher(kp1, kp1 + len[0]);

src/Desc.cc

@@ -164,7 +164,14 @@ void ODesc::Add(double d, bool no_exp)
 
 		Add(tmp);
 
-		if ( nearbyint(d) == d && isfinite(d) && ! strchr(tmp, 'e') )
+		auto approx_equal = [](double a, double b, double tolerance = 1e-6) -> bool
+			{
+			auto v = a - b;
+			return v < 0 ? -v < tolerance : v < tolerance;
+			};
+
+		if ( approx_equal(d, nearbyint(d), 1e-9) &&
+		     isfinite(d) && ! strchr(tmp, 'e') )
 			// disambiguate from integer
 			Add(".0");
 		}

src/Val.cc

@@ -13,6 +13,8 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+#include <cmath>
+
 #include "Attr.h"
 #include "BroString.h"
 #include "CompHash.h"
@@ -715,7 +717,8 @@ void IntervalVal::ValDescribe(ODesc* d) const
 			if ( ! (v >= unit || v <= -unit) )
 				continue;
 
-			double num = static_cast<double>(static_cast<int64_t>(v / unit));
+			double num = v / unit;
+			num = num < 0 ? std::ceil(num) : std::floor(num);
 			v -= num * unit;
 			to_print = num;
 			}

src/analyzer/protocol/http/HTTP.cc

@@ -472,14 +472,16 @@ void HTTP_Entity::SubmitHeader(mime::MIME_Header* h)
 
 	else if ( mime::istrequal(h->get_name(), "transfer-encoding") )
 		{
-		double http_version = 0;
+		HTTP_Analyzer::HTTP_VersionNumber http_version;
+
 		if (http_message->analyzer->GetRequestOngoing())
-			http_version = http_message->analyzer->GetRequestVersion();
+			http_version = http_message->analyzer->GetRequestVersionNumber();
 		else // reply_ongoing
-			http_version = http_message->analyzer->GetReplyVersion();
+			http_version = http_message->analyzer->GetReplyVersionNumber();
 
 		data_chunk_t vt = h->get_value_token();
-		if ( mime::istrequal(vt, "chunked") && http_version == 1.1 )
+		if ( mime::istrequal(vt, "chunked") &&
+		     http_version == HTTP_Analyzer::HTTP_VersionNumber{1, 1} )
 			chunked_transfer_state = BEFORE_CHUNK;
 		}
 
@@ -842,7 +844,6 @@ HTTP_Analyzer::HTTP_Analyzer(Connection* conn)
 	{
 	num_requests = num_replies = 0;
 	num_request_lines = num_reply_lines = 0;
-	request_version = reply_version = 0.0;	// unknown version
 	keep_alive = 0;
 	connection_close = 0;
 
@@ -1184,8 +1185,8 @@ void HTTP_Analyzer::GenStats()
 		RecordVal* r = new RecordVal(http_stats_rec);
 		r->Assign(0, val_mgr->GetCount(num_requests));
 		r->Assign(1, val_mgr->GetCount(num_replies));
-		r->Assign(2, new Val(request_version, TYPE_DOUBLE));
+		r->Assign(2, new Val(request_version.ToDouble(), TYPE_DOUBLE));
-		r->Assign(3, new Val(reply_version, TYPE_DOUBLE));
+		r->Assign(3, new Val(reply_version.ToDouble(), TYPE_DOUBLE));
 
 		// DEBUG_MSG("%.6f http_stats\n", network_time);
 		ConnectionEventFast(http_stats, {BuildConnVal(), r});
@@ -1318,14 +1319,14 @@ int HTTP_Analyzer::ParseRequest(const char* line, const char* end_of_line)
 	if ( version_start >= end_of_line )
 		{
 		// If no version is found
-		SetVersion(request_version, 0.9);
+		SetVersion(&request_version, {0, 9});
 		}
 	else
 		{
 		if ( version_start + 8 <= end_of_line )
 			{
 			version_start += 5; // "HTTP/"
-			SetVersion(request_version,
+			SetVersion(&request_version,
 					HTTP_Version(end_of_line - version_start,
 							version_start));
 
@@ -1348,31 +1349,33 @@ int HTTP_Analyzer::ParseRequest(const char* line, const char* end_of_line)
 	}
 
 // Only recognize [0-9][.][0-9].
-double HTTP_Analyzer::HTTP_Version(int len, const char* data)
+HTTP_Analyzer::HTTP_VersionNumber HTTP_Analyzer::HTTP_Version(int len, const char* data)
 	{
 	if ( len >= 3 &&
 	     data[0] >= '0' && data[0] <= '9' &&
 	     data[1] == '.' &&
 	     data[2] >= '0' && data[2] <= '9' )
 		{
-		return double(data[0] - '0') + 0.1 * double(data[2] - '0');
+		uint8_t major = data[0] - '0';
+		uint8_t minor = data[2] - '0';
+		return {major, minor};
 		}
 	else
 		{
 	        HTTP_Event("bad_HTTP_version", mime::new_string_val(len, data));
-		return 0;
+		return {};
 		}
 	}
 
-void HTTP_Analyzer::SetVersion(double& version, double new_version)
+void HTTP_Analyzer::SetVersion(HTTP_VersionNumber* version, HTTP_VersionNumber new_version)
 	{
-	if ( version == 0.0 )
+	if ( *version == HTTP_VersionNumber{} )
-		version = new_version;
+		*version = new_version;
 
-	else if ( version != new_version )
+	else if ( *version != new_version )
 		Weird("HTTP_version_mismatch");
 
-	if ( version > 1.05 )
+	if ( version->major > 1 || (version->major == 1 && version->minor > 0) )
 		keep_alive = 1;
 	}
 
@@ -1434,7 +1437,7 @@ void HTTP_Analyzer::HTTP_Request()
 			request_method,
 			TruncateURI(request_URI->AsStringVal()),
 			TruncateURI(unescaped_URI->AsStringVal()),
-			new StringVal(fmt("%.1f", request_version)),
+			new StringVal(fmt("%.1f", request_version.ToDouble())),
 		});
 		}
 	}
@@ -1445,7 +1448,7 @@ void HTTP_Analyzer::HTTP_Reply()
 		{
 		ConnectionEventFast(http_reply, {
 			BuildConnVal(),
-			new StringVal(fmt("%.1f", reply_version)),
+			new StringVal(fmt("%.1f", reply_version.ToDouble())),
 			val_mgr->GetCount(reply_code),
 			reply_reason_phrase ?
 				reply_reason_phrase->Ref() :
@@ -1565,7 +1568,7 @@ int HTTP_Analyzer::HTTP_ReplyLine(const char* line, const char* end_of_line)
 		return 0;
 		}
 
-	SetVersion(reply_version, HTTP_Version(end_of_line - rest, rest));
+	SetVersion(&reply_version, HTTP_Version(end_of_line - rest, rest));
 
 	for ( ; rest < end_of_line; ++rest )
 		if ( mime::is_lws(*rest) )

src/analyzer/protocol/http/HTTP.h

@@ -175,8 +175,24 @@ public:
 	void ConnectionReset() override;
 	void PacketWithRST() override;
 
-	double GetRequestVersion() { return request_version; };
+	struct HTTP_VersionNumber {
-	double GetReplyVersion() { return reply_version; };
+		uint8_t major = 0;
+		uint8_t minor = 0;
+
+		bool operator==(const HTTP_VersionNumber& other) const
+			{ return minor == other.minor && major == other.major; }
+
+		bool operator!=(const HTTP_VersionNumber& other) const
+			{ return ! operator==(other); }
+
+		double ToDouble() const
+			{ return major + minor * 0.1; }
+	};
+
+	double GetRequestVersion() { return request_version.ToDouble(); };
+	double GetReplyVersion() { return reply_version.ToDouble(); };
+	HTTP_VersionNumber GetRequestVersionNumber() { return request_version; };
+	HTTP_VersionNumber GetReplyVersionNumber() { return reply_version; };
 	int GetRequestOngoing() { return request_ongoing; };
 	int GetReplyOngoing() { return reply_ongoing; };
 
@@ -204,9 +220,9 @@ protected:
 				const char* prefix);
 
 	int ParseRequest(const char* line, const char* end_of_line);
-	double HTTP_Version(int len, const char* data);
+	HTTP_VersionNumber HTTP_Version(int len, const char* data);
 
-	void SetVersion(double& version, double new_version);
+	void SetVersion(HTTP_VersionNumber* version, HTTP_VersionNumber new_version);
 
 	int RequestExpected() const { return num_requests == 0 || keep_alive; }
 
@@ -227,7 +243,7 @@ protected:
 	int request_state, reply_state;
 	int num_requests, num_replies;
 	int num_request_lines, num_reply_lines;
-	double request_version, reply_version;
+	HTTP_VersionNumber request_version, reply_version;
 	int keep_alive;
 	int connection_close;
 	int request_ongoing, reply_ongoing;

src/broker/Manager.cc

@@ -625,7 +625,7 @@ void Manager::Error(const char* format, ...)
 	{
 	va_list args;
 	va_start(args, format);
-	auto msg = fmt(format, args);
+	auto msg = vfmt(format, args);
 	va_end(args);
 
 	if ( script_scope )

src/supervisor/Supervisor.cc

@@ -724,7 +724,7 @@ void Stem::ReportStatus(const Supervisor::Node& node) const
 
 void Stem::Log(std::string_view type, const char* format, va_list args) const
 	{
-	auto raw_msg = fmt(format, args);
+	auto raw_msg = vfmt(format, args);
 
 	if ( getenv("ZEEK_DEBUG_STEM_STDERR") )
 		{

src/util.cc

@@ -817,7 +817,7 @@ const char* fmt_bytes(const char* data, int len)
 	return buf;
 	}
 
-const char* fmt(const char* format, va_list al)
+const char* vfmt(const char* format, va_list al)
 	{
 	static char* buf = 0;
 	static unsigned int buf_len = 1024;
@@ -848,7 +848,7 @@ const char* fmt(const char* format, ...)
 	{
 	va_list al;
 	va_start(al, format);
-	auto rval = fmt(format, al);
+	auto rval = vfmt(format, al);
 	va_end(al);
 	return rval;
 	}

src/util.h

@@ -188,7 +188,7 @@ extern std::string strtolower(const std::string& s);
 extern const char* fmt_bytes(const char* data, int len);
 
 // Note: returns a pointer into a shared buffer.
-extern const char* fmt(const char* format, va_list args);
+extern const char* vfmt(const char* format, va_list args);
 // Note: returns a pointer into a shared buffer.
 extern const char* fmt(const char* format, ...)
 	__attribute__((format (printf, 1, 2)));

testing/btest/Baseline/core.print-interval/out

@@ -8,12 +8,12 @@
 1.0 day 10.0 hrs 17.0 mins 36.0 secs 789.0 msecs 123.449984 usecs
 -1.0 day -10.0 hrs -17.0 mins -36.0 secs -789.0 msecs -123.449984 usecs
 1.001 usecs
-1.0 msec 1 usec
+1.0 msec 1.0 usec
 11.0 msecs
 8.0 days 12.0 hrs
 7.0 hrs 30.0 mins
 6.0 mins 30.0 secs
 5.0 secs 500.0 msecs
-4.0 msecs 500 usecs
+4.0 msecs 500.0 usecs
 3.5 usecs
 2.0 days 2.0 secs

testing/btest/Baseline/scripts.base.frameworks.logging.ascii-double/test.log

@@ -22,8 +22,8 @@
 178999999999999996376899522972626047077637637819240219954027593177370961667659291027329061638406108931437333529420935752785895444161234074984843178962619172326295244262722141766382622299223626438470088150218987997954747866198184686628013966119769261150988554952970462018533787926725176560021258785656871583744.0
 -178999999999999996376899522972626047077637637819240219954027593177370961667659291027329061638406108931437333529420935752785895444161234074984843178962619172326295244262722141766382622299223626438470088150218987997954747866198184686628013966119769261150988554952970462018533787926725176560021258785656871583744.0
 0.000012
-0
+0.0
--0
+-0.0
 inf
 -inf
 0.0

testing/external/commit-hash.zeek-testing

@@ -1 +1 @@
-da47ae786562da18910d994b7868530929db6271
+96e78328a1a9e6c07e725b282c293728f70737f0