Libguestfs - Aug 2017 - [PATCH v2 supermin 1/1] Switch binary embedding to a C source

Hi,

this patch replaces the first simpler version:
https://www.redhat.com/archives/libguestfs/2017-August/msg00117.html

The approach now is to use a C snippet, which should pose way less
compatibility issues.

Thanks,

Pino Toscano (1):
  Switch binary embedding to a C source

 .gitignore                 |  2 +-
 src/Makefile.am            | 18 +++++++++---------
 src/{bin2s.pl => bin2c.pl} | 24 +++++++++---------------
 src/format-ext2-init-c.c   |  9 ++++-----
 4 files changed, 23 insertions(+), 30 deletions(-)
 rename src/{bin2s.pl => bin2c.pl} (69%)

-- 
2.13.5

The current approach of writing a custom assembler snippet has few
issues:
- the assembler needs to manually things like the correct section,
  disabling the executable stack, and so on
- the symbols are global, and exported with an awkward interface
  (i.e. uint8_t offsets)
- the assembler source is built with a manual rule
- the embedded data is not properly aligned according to the needs of
  the platform
- in the past, the snippet had also tweaks for specific architectures
  (e.g. HPPA)

To overcome this situation, turn the bin2s.pl script into a bin2c.pl,
which instead generates a C snippet containing a C array with the data,
and a variable with the length of the data.  The code produced this way
has various advantages:
- it is simple C code, with no need for architecture tweaks
- it is constant, so the compiler places it in the rodata section, with
  the correct alignment
- it is static, so no symbols are exported
- it is included directly into the source using it, so no need for
  manual compilation rules

This is inspired by what the xxd tool (part of vim) does, although it is
less flexible than needed.
---
 .gitignore                 |  2 +-
 src/Makefile.am            | 18 +++++++++---------
 src/{bin2s.pl => bin2c.pl} | 24 +++++++++---------------
 src/format-ext2-init-c.c   |  9 ++++-----
 4 files changed, 23 insertions(+), 30 deletions(-)
 rename src/{bin2s.pl => bin2c.pl} (69%)

diff --git a/.gitignore b/.gitignore
index 0983349..918b2f4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -40,7 +40,7 @@ pod2htm?.tmp
 /snippet/
 /src/.depend
 /src/config.ml
-/src/format-ext2-init-bin.S
+/src/format-ext2-init-bin.h
 /src/supermin
 /src/supermin.1
 /src/supermin-link.sh
diff --git a/src/Makefile.am b/src/Makefile.am
index 5670a0d..d897a5b 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -17,7 +17,7 @@
 
 EXTRA_DIST = \
 	.depend \
-	bin2s.pl \
+	bin2c.pl \
 	config.ml.in \
 	supermin.1 \
 	supermin.pod \
@@ -111,11 +111,13 @@ man_MANS = \
 bin_PROGRAMS = supermin
 
 supermin_SOURCES = $(SOURCES_C)
+nodist_supermin_SOURCES = format-ext2-init-bin.h
 supermin_CFLAGS = \
 	-I$(shell $(OCAMLC) -where) \
 	$(EXT2FS_CFLAGS) $(COM_ERR_CFLAGS) $(LIBRPM_CFLAGS) \
 	-Wall $(WERROR_CFLAGS) \
 	-I$(top_srcdir)/lib -I../lib
+format-ext2-init-c.$(OBJEXT): format-ext2-init-bin.h
 
 BOBJECTS = $(SOURCES_ML:.ml=.cmo)
 XOBJECTS = $(SOURCES_ML:.ml=.cmx)
@@ -132,9 +134,9 @@ OBJECTS = $(XOBJECTS)
 BEST    = opt
 endif
 
-supermin_DEPENDENCIES = $(OBJECTS) format-ext2-init-bin.o
+supermin_DEPENDENCIES = $(OBJECTS)
 
-supermin_LDADD = format-ext2-init-bin.o ../lib/libgnu.a
+supermin_LDADD = ../lib/libgnu.a
 
 supermin_LINK = \
 	./supermin-link.sh \
@@ -148,17 +150,15 @@ supermin_LINK = \
 .ml.cmx:
 	$(OCAMLFIND) ocamlopt $(OCAMLFLAGS) $(OCAMLPACKAGES) -c $< -o $@
 
-CLEANFILES += format-ext2-init-bin.S
+CLEANFILES += format-ext2-init-bin.h
+BUILT_SOURCES = format-ext2-init-bin.h
 
-format-ext2-init-bin.o: format-ext2-init-bin.S
-	$(CC) -o $@ -c $<
-
-format-ext2-init-bin.S: ../init/init $(srcdir)/bin2s.pl
+format-ext2-init-bin.h: ../init/init $(srcdir)/bin2c.pl
 	strip --strip-all $<
 	ls -l $<
 	@file $< | grep -isq static || \
 	  (echo "*** error: init is not statically linked"; exit 1)
-	$(srcdir)/bin2s.pl $< $@
+	$(srcdir)/bin2c.pl $< $@
 
 depend: .depend
 
diff --git a/src/bin2s.pl b/src/bin2c.pl
similarity index 69%
rename from src/bin2s.pl
rename to src/bin2c.pl
index 6c70446..b3f94ec 100755
--- a/src/bin2s.pl
+++ b/src/bin2c.pl
@@ -15,10 +15,11 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
-# This script creates a source file for the GNU assembler which shuold
-# result in an object file equivalent to that of
+# This script creates a C snippet embedding an arbitrary file
 #
-# objcopy -I binary -B $(DEFAULT_ARCH) -O $(ELF_DEFAULT_ARCH) <in>
<out>
+# The output provides two variables:
+# static const char _binary_$name[];
+# static const size_t _binary_$name_len;
 
 use strict;
 use warnings;
@@ -36,28 +37,21 @@ $infile_basename =~ s{.*/}{};
 print $ofh <<"EOF";
 /* This file has been automatically generated from $infile by $0 */
 
-/* Mark stack as non-executable for GNU tools. */
-\t.section .note.GNU-stack,"",%progbits
-\t.previous
-
-\t.globl\t_binary_${infile_basename}_start
-\t.globl\t_binary_${infile_basename}_end
-
-\t.section\t.rodata
-_binary_${infile_basename}_start:
+static const char _binary_${infile_basename}[] = {
 EOF
 
 $sz = 0;
 while ( $i = read $ifh, $buf, 12 ) {
-    print $ofh "\t.byte\t"
-      . join( ',', map { sprintf '0x%02x', ord $_ } split //,
$buf ) . "\n";
+    print $ofh "  "
+      . join( ", ", map { sprintf '0x%02x', ord $_ } split
//, $buf ) . ",\n";
     $sz += $i;
 }
 die "read $infile (at offset $sz): $!\n" if not defined $i;
 close $ifh;
 
 print $ofh <<"EOF";
-_binary_${infile_basename}_end:
+};
+static const size_t _binary_${infile_basename}_len = ${sz};
 EOF
 
 close $ofh;
diff --git a/src/format-ext2-init-c.c b/src/format-ext2-init-c.c
index 66ad254..33147aa 100644
--- a/src/format-ext2-init-c.c
+++ b/src/format-ext2-init-c.c
@@ -27,19 +27,18 @@
 #include <caml/memory.h>
 
 /* The init binary.
- * See: bin2s.pl, init.c.
+ * See: bin2c.pl, init.c.
  */
-extern uint8_t _binary_init_start, _binary_init_end;
+#include <format-ext2-init-bin.h>
 
 value
 supermin_binary_init (value unitv)
 {
   CAMLparam1 (unitv);
   CAMLlocal1 (sv);
-  size_t n = &_binary_init_end - &_binary_init_start;
 
-  sv = caml_alloc_string (n);
-  memcpy (String_val (sv), &_binary_init_start, n);
+  sv = caml_alloc_string (_binary_init_len);
+  memcpy (String_val (sv), _binary_init, _binary_init_len);
 
   CAMLreturn (sv);
 }
-- 
2.13.5

ACK - this seems a better approach.

My concern was that it might make the generated binary bigger for some
reason, but I checked and the stripped binary is identical in size
(unstripped is actually smaller although I'm unclear why).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW