[PATCH 0/3] mwrap odds and ends before 2.3..

[PATCH 0/3] mwrap odds and ends before 2.3..

[Eric Wong]

Not sure why rdoc scans extconf.rb during `gem install'
--version + --help may be useful for new users; and a
failed attempt at getting FreeBSD support working, but
probably leaves things in a nicer state if we attempt it
again...

Eric Wong (3):
  extconf.rb: avoid RDoc errors during gem install
  add --version and --help args
  cleanup some FreeBSD-related workarounds

 MANIFEST             |  3 ++
 README               |  2 +-
 VERSION-GEN          | 36 +++++++++++++++++
 bin/mwrap            | 19 +++++++++
 ext/mwrap/extconf.rb |  8 ++--
 ext/mwrap/mwrap.c    | 93 +++++++++++++++++++++++++-------------------
 lib/mwrap/.gitignore |  1 +
 mwrap.gemspec        |  9 +++--
 8 files changed, 123 insertions(+), 48 deletions(-)
 create mode 100755 VERSION-GEN
 create mode 100644 lib/mwrap/.gitignore

[PATCH 1/3] extconf.rb: avoid RDoc errors during gem install

[Eric Wong]

RDoc doesn't seem to like <<'' (blank line) as a heredoc
terminator.  That said, RDoc scanning extconf.rb during "gem install"
is unexpected and make no sense to me.  Normal `rdoc' invocations
seem to ignore extconf.rb.
---
 ext/mwrap/extconf.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ext/mwrap/extconf.rb b/ext/mwrap/extconf.rb
index 512ab82..1828407 100644
--- a/ext/mwrap/extconf.rb
+++ b/ext/mwrap/extconf.rb
@@ -11,15 +11,15 @@ have_library 'dl'
 have_library 'c'
 have_library 'execinfo' # FreeBSD
 
-if try_link(<<'')
+if try_link(<<EOC)
 int main(void) { return __builtin_add_overflow_p(0,0,(int)1); }
-
+EOC
   $defs << '-DHAVE_BUILTIN_ADD_OVERFLOW_P'
 end
 
-if try_link(<<'')
+if try_link(<<EOC)
 int main(int a) { return __builtin_add_overflow(0,0,&a); }
-
+EOC
   $defs << '-DHAVE_BUILTIN_ADD_OVERFLOW_P'
 else
   abort 'missing __builtin_add_overflow'

[PATCH 2/3] add --version and --help args

[Eric Wong]

These may make things easier for new users, and we'll also with
help text if given no args.

We'll programmatically generate version based on `git describe',
but fallback to a hardcoded version if outside of git.  We'll
also start appending `-dirty' to the version string to match
git.git conventions.
---
 MANIFEST             |  3 +++
 VERSION-GEN          | 36 ++++++++++++++++++++++++++++++++++++
 bin/mwrap            | 19 +++++++++++++++++++
 lib/mwrap/.gitignore |  1 +
 mwrap.gemspec        |  9 ++++++---
 5 files changed, 65 insertions(+), 3 deletions(-)
 create mode 100755 VERSION-GEN
 create mode 100644 lib/mwrap/.gitignore

diff --git a/MANIFEST b/MANIFEST
index e6d8964..e8ace8b 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -5,10 +5,13 @@ COPYING
 MANIFEST
 README
 Rakefile
+VERSION-GEN
 bin/mwrap
 ext/mwrap/extconf.rb
 ext/mwrap/jhash.h
 ext/mwrap/mwrap.c
+lib/mwrap/.gitignore
 lib/mwrap_rack.rb
 mwrap.gemspec
 test/test_mwrap.rb
+lib/mwrap/version.rb
diff --git a/VERSION-GEN b/VERSION-GEN
new file mode 100755
index 0000000..161a04f
--- /dev/null
+++ b/VERSION-GEN
@@ -0,0 +1,36 @@
+#!/bin/sh
+VF=lib/mwrap/version.rb
+DEF_VER=v2.2.0
+VN=$(git describe HEAD 2>/dev/null)
+if test $? -eq 0
+then
+	case "$VN" in
+	v[0-9]*)
+		set -e
+		git update-index -q --refresh
+		set +e
+		git diff-index --quiet HEAD -- || VN="$VN-dirty"
+		set -e
+		VN=$(echo $VN | tr '-' '.')
+		;;
+	esac
+fi
+set -e
+
+case $VN in
+'') VN="$DEF_VER" ;;
+esac
+
+VN=$(expr "$VN" : v*'\(.*\)')
+VC=unset
+if test -r $VF
+then
+	VC="$(cat $VF)"
+fi
+
+new="module Mwrap; VERSION = '$VN'.freeze; end"
+if test x"$new" != x"$VC"
+then
+	echo "$new" >$VF
+fi
+echo $VN
diff --git a/bin/mwrap b/bin/mwrap
index 212078c..054b3a3 100755
--- a/bin/mwrap
+++ b/bin/mwrap
@@ -2,6 +2,25 @@
 # frozen_string_literal: true
 # Copyright (C) mwrap hackers <mwrap-public@80x24.org>
 # License: GPL-2.0+ <https://www.gnu.org/licenses/gpl-2.0.txt>
+help = <<EOM
+usage: mwrap COMMAND [ARGS]
+see https://80x24.org/mwrap/README.html for more info
+EOM
+ARGV.empty? and abort help
+ARGV.each do |x|
+  case x
+  when '--version', '-v'
+    require 'mwrap/version'
+    puts "mwrap #{Mwrap::VERSION} - #{RUBY_DESCRIPTION}"
+    exit 0
+  when '--help', '-h'
+    puts help
+    exit 0
+  else # don't intercept --version/--help intended for commands we wrap
+    break
+  end
+end
+
 require 'mwrap'
 mwrap_so = $".grep(%r{/mwrap\.so\z})[0] or abort "mwrap.so not loaded"
 cur = ENV['LD_PRELOAD']
diff --git a/lib/mwrap/.gitignore b/lib/mwrap/.gitignore
new file mode 100644
index 0000000..07c0394
--- /dev/null
+++ b/lib/mwrap/.gitignore
@@ -0,0 +1 @@
+version.rb
diff --git a/mwrap.gemspec b/mwrap.gemspec
index 48a32b2..cb541e3 100644
--- a/mwrap.gemspec
+++ b/mwrap.gemspec
@@ -1,18 +1,21 @@
 git_manifest = `git ls-files 2>/dev/null`.split("\n")
+git_ok = $?.success?
+git_manifest << 'lib/mwrap/version.rb'.freeze # generated by ./VERSION-GEN
 manifest = File.exist?('MANIFEST') ?
   File.readlines('MANIFEST').map!(&:chomp).delete_if(&:empty?) : git_manifest
-if git_manifest[0] && manifest != git_manifest
+if git_ok && manifest != git_manifest
   tmp = "MANIFEST.#$$.tmp"
   File.open(tmp, 'w') { |fp| fp.puts(git_manifest.join("\n")) }
   File.rename(tmp, 'MANIFEST')
   system('git add MANIFEST')
 end
 
-desc = `git describe --abbrev=4 HEAD`.strip.tr('-', '.').delete_prefix('v')
+version = `./VERSION-GEN`.chomp
+$?.success? or abort './VERSION-GEN failed'
 
 Gem::Specification.new do |s|
   s.name = 'mwrap'
-  s.version = desc.empty? ? '2.2.0' : desc
+  s.version = version
   s.homepage = 'https://80x24.org/mwrap/'
   s.authors = ["mwrap hackers"]
   s.summary = 'LD_PRELOAD malloc wrapper for Ruby'

[PATCH 3/3] cleanup some FreeBSD-related workarounds

[Eric Wong]

While FreeBSD 12.3 + Ruby 3.0.4p208 remains broken with mwrap
(likely due to threading bugs in Ruby) at least get the code
more consistently closer to a working state.

Thus we'll remember to account for PTHREAD_MUTEX_INITIALIZER
requiring malloc for HeapPageBody tracking, as we do with other
mutexes.

For reference, the Perl5 port of mwrap seems to have no problems
on FreeBSD 12.3, so it seems down to misbehavior w.r.t. pthreads
usage within Ruby itself.  Perl5 on FreeBSD is configured with
threads support, but Perl5 doesn't spawn background threads by
default like Ruby can.
---
 README            |  2 +-
 ext/mwrap/mwrap.c | 93 +++++++++++++++++++++++++++--------------------
 2 files changed, 54 insertions(+), 41 deletions(-)

diff --git a/README b/README
index f387bc4..539073e 100644
--- a/README
+++ b/README
@@ -23,7 +23,7 @@ It does not require recompiling or rebuilding Ruby, but only
 supports Ruby trunk (2.6.0dev+) on a few platforms:
 
 * GNU/Linux
-* FreeBSD (tested 11.1)
+* FreeBSD (tested 11.1 on Ruby 2.6, currently broken with Ruby 3.x)
 
 It may work on NetBSD, OpenBSD and DragonFly BSD.
 
diff --git a/ext/mwrap/mwrap.c b/ext/mwrap/mwrap.c
index 9d90298..1d6baec 100644
--- a/ext/mwrap/mwrap.c
+++ b/ext/mwrap/mwrap.c
@@ -99,9 +99,43 @@ union padded_mutex {
 /* a round-robin pool of mutexes */
 #define MUTEX_NR   (1 << 6)
 #define MUTEX_MASK (MUTEX_NR - 1)
+#ifdef __FreeBSD__
+#  define STATIC_MTX_INIT_OK (0)
+#else /* only tested on Linux + glibc */
+#  define STATIC_MTX_INIT_OK (1)
+#endif
 static size_t mutex_i;
 static union padded_mutex mutexes[MUTEX_NR] = {
+#if STATIC_MTX_INIT_OK
 	[0 ... (MUTEX_NR-1)].mtx = PTHREAD_MUTEX_INITIALIZER
+#endif
+};
+
+#define ACC_INIT(name) { .nr=0, .min=INT64_MAX, .max=-1, .m2=0, .mean=0 }
+struct acc {
+	uint64_t nr;
+	int64_t min;
+	int64_t max;
+	double m2;
+	double mean;
+};
+
+/* for tracking 16K-aligned heap page bodies (protected by GVL) */
+struct {
+	pthread_mutex_t lock;
+	struct cds_list_head bodies;
+	struct cds_list_head freed;
+
+	struct acc alive;
+	struct acc reborn;
+} hpb_stats = {
+#if STATIC_MTX_INIT_OK
+	.lock = PTHREAD_MUTEX_INITIALIZER,
+#endif
+	.bodies = CDS_LIST_HEAD_INIT(hpb_stats.bodies),
+	.freed = CDS_LIST_HEAD_INIT(hpb_stats.freed),
+	.alive = ACC_INIT(hpb_stats.alive),
+	.reborn = ACC_INIT(hpb_stats.reborn)
 };
 
 static pthread_mutex_t *mutex_assign(void)
@@ -120,12 +154,11 @@ __attribute__((constructor)) static void resolve_malloc(void)
 	int err;
 	++locating;
 
-#ifdef __FreeBSD__
 	/*
 	 * PTHREAD_MUTEX_INITIALIZER on FreeBSD means lazy initialization,
 	 * which happens at pthread_mutex_lock, and that calls calloc
 	 */
-	{
+	if (!STATIC_MTX_INIT_OK) {
 		size_t i;
 
 		for (i = 0; i < MUTEX_NR; i++) {
@@ -135,22 +168,28 @@ __attribute__((constructor)) static void resolve_malloc(void)
 				_exit(1);
 			}
 		}
+		err = pthread_mutex_init(&hpb_stats.lock, 0);
+		if (err) {
+			fprintf(stderr, "error: %s\n", strerror(err));
+			_exit(1);
+		}
 		/* initialize mutexes used by urcu-bp */
 		rcu_read_lock();
 		rcu_read_unlock();
+#ifndef __FreeBSD__
+	} else {
+		if (!real_malloc) {
+			resolving_malloc = 1;
+			real_malloc = dlsym(RTLD_NEXT, "malloc");
+		}
+		real_free = dlsym(RTLD_NEXT, "free");
+		if (!real_malloc || !real_free) {
+			fprintf(stderr, "missing malloc/aligned_alloc/free\n"
+				"\t%p %p\n", real_malloc, real_free);
+			_exit(1);
+		}
+#endif /* !__FreeBSD__ */
 	}
-#else /* !FreeBSD (tested on GNU/Linux) */
-	if (!real_malloc) {
-		resolving_malloc = 1;
-		real_malloc = dlsym(RTLD_NEXT, "malloc");
-	}
-	real_free = dlsym(RTLD_NEXT, "free");
-	if (!real_malloc || !real_free) {
-		fprintf(stderr, "missing malloc/aligned_alloc/free\n"
-			"\t%p %p\n", real_malloc, real_free);
-		_exit(1);
-	}
-#endif /* !FreeBSD */
 	CMM_STORE_SHARED(totals, lfht_new());
 	if (!CMM_LOAD_SHARED(totals))
 		fprintf(stderr, "failed to allocate totals table\n");
@@ -237,32 +276,6 @@ static int has_ec_p(void)
 		ruby_current_vm_ptr && ruby_current_ec;
 }
 
-struct acc {
-	uint64_t nr;
-	int64_t min;
-	int64_t max;
-	double m2;
-	double mean;
-};
-
-#define ACC_INIT(name) { .nr=0, .min=INT64_MAX, .max=-1, .m2=0, .mean=0 }
-
-/* for tracking 16K-aligned heap page bodies (protected by GVL) */
-struct {
-	pthread_mutex_t lock;
-	struct cds_list_head bodies;
-	struct cds_list_head freed;
-
-	struct acc alive;
-	struct acc reborn;
-} hpb_stats = {
-	.lock = PTHREAD_MUTEX_INITIALIZER,
-	.bodies = CDS_LIST_HEAD_INIT(hpb_stats.bodies),
-	.freed = CDS_LIST_HEAD_INIT(hpb_stats.freed),
-	.alive = ACC_INIT(hpb_stats.alive),
-	.reborn = ACC_INIT(hpb_stats.reborn)
-};
-
 /* allocated via real_malloc/real_free */
 struct src_loc {
 	pthread_mutex_t *mtx;