[PATCH 0/4] Refactor usage of gcc -ffile-prefix-map

[PATCH 0/4] Refactor usage of gcc -ffile-prefix-map

[Juro Bystricky]

The gcc that comes with OE-core supports a compiler option -ffile-prefix-map.
This option is essential in order to support binary reproducibility of packages
built from sources containing absolute paths due to various __FILE__ macros.
These can contain absolute path names of source code files as encountered on
the build host machine.
This affects basically all 150+ packaged kernel-module*, out of tree modules
(everybody inheriting module.bbclass, i.e. lttng-modules) and potentially individual
recipes.

While this compiler options is very useful, the presence cannot be assumed as
we may be building using an external toolchain. The detection code was already present
in kernel.bbclass. Since we may want to use this option (if present) in additional places,
it makes sense putting the detection code in a separate bbclass as opposed to replicating it
each time.
Not being aware of any better place, I put the code into a new bbclass "compiler-options.bbclass"

This patchset refactors kernel.bbclass and module.bbclass to use the new compiler-options.bbclass.
For a good measure the patcheset also contains a modified recipe for pulsaudio, demonstrating
how to use the compiler-options in a recipe.


Juro Bystricky (4):
  compiler-options.bbclass: provide and validate options
  kernel.bbclass: use class compiler-options
  module.bbclass: improve reproducibility
  pulseaudio: improve reproducibility

 meta/classes/compiler-options.bbclass              | 24 +++++++
 meta/classes/kernel.bbclass                        | 15 +----
 meta/classes/module.bbclass                        |  5 +-
 .../pulseaudio/pulseaudio/no-builddir.patch        | 74 ++++++++++++++++++++++
 .../pulseaudio/pulseaudio_11.1.bb                  | 12 +++-
 5 files changed, 115 insertions(+), 15 deletions(-)
 create mode 100644 meta/classes/compiler-options.bbclass
 create mode 100644 meta/recipes-multimedia/pulseaudio/pulseaudio/no-builddir.patch

-- 
2.7.4

[PATCH 1/4] compiler-options.bbclass: provide and validate options

[Juro Bystricky]

As we may be using external toolchains, we cannot assume
various compiler options to be valid.

Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
---
 meta/classes/compiler-options.bbclass | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 meta/classes/compiler-options.bbclass

diff --git a/meta/classes/compiler-options.bbclass b/meta/classes/compiler-options.bbclass
new file mode 100644
index 0000000..fdd45e8
--- /dev/null
+++ b/meta/classes/compiler-options.bbclass
@@ -0,0 +1,24 @@
+
+# Some compiler options are not universal and using them when not
+# supported can cause build errors.
+#
+# Check for a compiler option -ffile-prefix-map.
+# This option is present in OE-core GCC, but may not be present when
+# using external toolchains.
+#
+# The option can remap any hard-coded __FILE__ typically containing build
+# host path to a reproducible value, with the absolute build-host path
+# stripped. This is essential in order to achieve reproducible builds.
+#
+# Typical usage:
+#
+#    inherit compiler-options
+#    cc_extra=$(file_prefix_map_option_supported ${CC})
+#    oe_runmake CFLAGS="${CFLAGS} $cc_extra"
+#
+
+file_prefix_map_option_supported () {
+	if $1 -Q --help=joined | grep -q "\-ffile-prefix-map=<old=new>"; then
+		echo "-ffile-prefix-map=${S}=/${BP}/"
+	fi
+}
-- 
2.7.4

[PATCH 2/4] kernel.bbclass: use class compiler-options

[Juro Bystricky]

We use gcc option "-ffile-prefix-map" if available.
Remove the local code that detects the availability of this
compiler option (get_cc_option) and use a more generic code
imported from compiler-option.bbclass.

Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
---
 meta/classes/kernel.bbclass | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index 7ef4f47..32730b1 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -1,4 +1,4 @@
-inherit linux-kernel-base kernel-module-split
+inherit linux-kernel-base kernel-module-split compiler-options
 
 PROVIDES += "virtual/kernel"
 DEPENDS += "virtual/${TARGET_PREFIX}binutils virtual/${TARGET_PREFIX}gcc kmod-native bc-native lzop-native"
@@ -244,15 +244,6 @@ python do_devshell_prepend () {
 
 addtask bundle_initramfs after do_install before do_deploy
 
-get_cc_option () {
-		# Check if KERNEL_CC supports the option "file-prefix-map".
-		# This option allows us to build images with __FILE__ values that do not
-		# contain the host build path.
-		if ${KERNEL_CC} -Q --help=joined | grep -q "\-ffile-prefix-map=<old=new>"; then
-			echo "-ffile-prefix-map=${S}=/kernel-source/"
-		fi
-}
-
 kernel_do_compile() {
 	unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
 	if [ "$BUILD_REPRODUCIBLE_BINARIES" = "1" ]; then
@@ -287,7 +278,7 @@ kernel_do_compile() {
 		copy_initramfs
 		use_alternate_initrd=CONFIG_INITRAMFS_SOURCE=${B}/usr/${INITRAMFS_IMAGE_NAME}.cpio
 	fi
-	cc_extra=$(get_cc_option)
+	cc_extra=$(file_prefix_map_option_supported ${KERNEL_CC})
 	for typeformake in ${KERNEL_IMAGETYPE_FOR_MAKE} ; do
 		oe_runmake ${typeformake} CC="${KERNEL_CC} $cc_extra " LD="${KERNEL_LD}" ${KERNEL_EXTRA_ARGS} $use_alternate_initrd
 	done
@@ -301,7 +292,7 @@ kernel_do_compile() {
 do_compile_kernelmodules() {
 	unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS MACHINE
 	if (grep -q -i -e '^CONFIG_MODULES=y$' ${B}/.config); then
-		cc_extra=$(get_cc_option)
+		cc_extra=$(file_prefix_map_option_supported ${KERNEL_CC})
 		oe_runmake -C ${B} ${PARALLEL_MAKE} modules CC="${KERNEL_CC} $cc_extra " LD="${KERNEL_LD}" ${KERNEL_EXTRA_ARGS}
 
 		# Module.symvers gets updated during the 
-- 
2.7.4

[PATCH 3/4] module.bbclass: improve reproducibility

[Juro Bystricky]

Use gcc option "-ffile-prefix-map" if available.
This will remap any hard-coded __FILE__ containing build host
path to a reproducible value, with the absolute build-host path
stripped.

Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
---
 meta/classes/module.bbclass | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/meta/classes/module.bbclass b/meta/classes/module.bbclass
index 78d1b21..e0670bf 100644
--- a/meta/classes/module.bbclass
+++ b/meta/classes/module.bbclass
@@ -1,4 +1,4 @@
-inherit module-base kernel-module-split pkgconfig
+inherit module-base kernel-module-split pkgconfig compiler-options
 
 addtask make_scripts after do_prepare_recipe_sysroot before do_configure
 do_make_scripts[lockfiles] = "${TMPDIR}/kernel-scripts.lock"
@@ -40,9 +40,10 @@ python do_devshell_prepend () {
 
 module_do_compile() {
 	unset CFLAGS CPPFLAGS CXXFLAGS LDFLAGS
+	cc_extra=$(file_prefix_map_option_supported ${KERNEL_CC})
 	oe_runmake KERNEL_PATH=${STAGING_KERNEL_DIR}   \
 		   KERNEL_VERSION=${KERNEL_VERSION}    \
-		   CC="${KERNEL_CC}" LD="${KERNEL_LD}" \
+		   CC="${KERNEL_CC} $cc_extra" LD="${KERNEL_LD}" \
 		   AR="${KERNEL_AR}" \
 	           O=${STAGING_KERNEL_BUILDDIR} \
 		   KBUILD_EXTRA_SYMBOLS="${KBUILD_EXTRA_SYMBOLS}" \
-- 
2.7.4

[PATCH 4/4] pulseaudio: improve reproducibility

[Juro Bystricky]

1. Remove build host references from CFLAGS in generated config.h file.
   They end up compiled in the image, but are only used for information.
2. Remove PA_BUILDDIR hardcoded references: safe to remove as we are guaranteed
   not to run from the build directory when the image is cross-compiled
3. Take care of any embedded __FILE__ strings, but only if the compiler supports
   -ffile-prefix-map option.

Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
---
 .../pulseaudio/pulseaudio/no-builddir.patch        | 74 ++++++++++++++++++++++
 .../pulseaudio/pulseaudio_11.1.bb                  | 12 +++-
 2 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-multimedia/pulseaudio/pulseaudio/no-builddir.patch

diff --git a/meta/recipes-multimedia/pulseaudio/pulseaudio/no-builddir.patch b/meta/recipes-multimedia/pulseaudio/pulseaudio/no-builddir.patch
new file mode 100644
index 0000000..fcef858
--- /dev/null
+++ b/meta/recipes-multimedia/pulseaudio/pulseaudio/no-builddir.patch
@@ -0,0 +1,74 @@
+Improve reproducibility.
+Remove hard coded build host references (PA_BUILDDIR) from the cross-compiled
+image. For cross-compiled images we are guaranteed not to run from build directory.
+
+Upstream-Status: Inappropriate [embedded specific]
+Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
+
+diff --git a/src/daemon/daemon-conf.c b/src/daemon/daemon-conf.c
+index f0ed0b4..94a56d3 100644
+--- a/src/daemon/daemon-conf.c
++++ b/src/daemon/daemon-conf.c
+@@ -153,11 +153,7 @@ pa_daemon_conf *pa_daemon_conf_new(void) {
+     c->dl_search_path = pa_sprintf_malloc("%s" PA_PATH_SEP "lib" PA_PATH_SEP "pulse-%d.%d" PA_PATH_SEP "modules",
+                                           pa_win32_get_toplevel(NULL), PA_MAJOR, PA_MINOR);
+ #else
+-    if (pa_run_from_build_tree()) {
+-        pa_log_notice("Detected that we are run from the build tree, fixing search path.");
+-        c->dl_search_path = pa_xstrdup(PA_BUILDDIR);
+-    } else
+-        c->dl_search_path = pa_xstrdup(PA_DLSEARCHPATH);
++    c->dl_search_path = pa_xstrdup(PA_DLSEARCHPATH);
+ #endif
+ 
+     return c;
+diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
+index 3dbf6b1..98cb738 100644
+--- a/src/modules/alsa/alsa-mixer.c
++++ b/src/modules/alsa/alsa-mixer.c
+@@ -2505,10 +2505,7 @@ static int path_verify(pa_alsa_path *p) {
+ }
+ 
+ static const char *get_default_paths_dir(void) {
+-    if (pa_run_from_build_tree())
+-        return PA_SRCDIR "/modules/alsa/mixer/paths/";
+-    else
+-        return PA_ALSA_PATHS_DIR;
++    return PA_ALSA_PATHS_DIR;
+ }
+ 
+ pa_alsa_path* pa_alsa_path_new(const char *paths_dir, const char *fname, pa_alsa_direction_t direction) {
+@@ -4383,9 +4380,7 @@ pa_alsa_profile_set* pa_alsa_profile_set_new(const char *fname, const pa_channel
+     if (!fname)
+         fname = "default.conf";
+ 
+-    fn = pa_maybe_prefix_path(fname,
+-                              pa_run_from_build_tree() ? PA_SRCDIR "/modules/alsa/mixer/profile-sets/" :
+-                              PA_ALSA_PROFILE_SETS_DIR);
++    fn = pa_maybe_prefix_path(fname, PA_ALSA_PROFILE_SETS_DIR);
+ 
+     r = pa_config_parse(fn, NULL, items, NULL, false, ps);
+     pa_xfree(fn);
+diff --git a/src/pulsecore/core-util.c b/src/pulsecore/core-util.c
+index 873f86a..937bac6 100644
+--- a/src/pulsecore/core-util.c
++++ b/src/pulsecore/core-util.c
+@@ -3397,17 +3397,7 @@ void pa_reset_personality(void) {
+ }
+ 
+ bool pa_run_from_build_tree(void) {
+-    char *rp;
+-    static bool b = false;
+-
+-    PA_ONCE_BEGIN {
+-        if ((rp = pa_readlink("/proc/self/exe"))) {
+-            b = pa_startswith(rp, PA_BUILDDIR);
+-            pa_xfree(rp);
+-        }
+-    } PA_ONCE_END;
+-
+-    return b;
++    return false;
+ }
+ 
+ const char *pa_get_temp_dir(void) {
diff --git a/meta/recipes-multimedia/pulseaudio/pulseaudio_11.1.bb b/meta/recipes-multimedia/pulseaudio/pulseaudio_11.1.bb
index a50b5a0..c9c2d2d 100644
--- a/meta/recipes-multimedia/pulseaudio/pulseaudio_11.1.bb
+++ b/meta/recipes-multimedia/pulseaudio/pulseaudio_11.1.bb
@@ -5,10 +5,20 @@ SRC_URI = "http://freedesktop.org/software/pulseaudio/releases/${BP}.tar.xz \
            file://0001-client-conf-Add-allow-autospawn-for-root.patch \
            file://volatiles.04_pulse \
 "
+
+SRC_URI_append_class-target = " \
+           file://no-builddir.patch \
+"
+
 SRC_URI[md5sum] = "390de38231d5cdd6b43ada8939eb74f1"
 SRC_URI[sha256sum] = "f2521c525a77166189e3cb9169f75c2ee2b82fa3fcf9476024fbc2c3a6c9cd9e"
 
-do_compile_prepend() {
+inherit compiler-options
+
+do_compile() {
     mkdir -p ${S}/libltdl
+    sed -i -e 's|${DEBUG_PREFIX_MAP}||g' ${B}/config.h
     cp ${STAGING_LIBDIR}/libltdl* ${S}/libltdl
+    cc_extra=$(file_prefix_map_option_supported ${CC})
+    oe_runmake CFLAGS="${CFLAGS} $cc_extra"
 }
-- 
2.7.4

Re: [PATCH 1/4] compiler-options.bbclass: provide and validate options

[Khem Raj]

On 12/13/17 3:31 PM, Juro Bystricky wrote:
> As we may be using external toolchains, we cannot assume
> various compiler options to be valid.
> 

While looking through my backlog :),  I think the idea is fine but
implementation can be a bit more generic. e.g. a common function to
probe the compiler for a given option then adding/removing it from
compiler commandline.

> Signed-off-by: Juro Bystricky <juro.bystricky@intel.com>
> ---
>  meta/classes/compiler-options.bbclass | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 meta/classes/compiler-options.bbclass
> 
> diff --git a/meta/classes/compiler-options.bbclass b/meta/classes/compiler-options.bbclass
> new file mode 100644
> index 0000000..fdd45e8
> --- /dev/null
> +++ b/meta/classes/compiler-options.bbclass
> @@ -0,0 +1,24 @@
> +
> +# Some compiler options are not universal and using them when not
> +# supported can cause build errors.
> +#
> +# Check for a compiler option -ffile-prefix-map.
> +# This option is present in OE-core GCC, but may not be present when
> +# using external toolchains.
> +#
> +# The option can remap any hard-coded __FILE__ typically containing build
> +# host path to a reproducible value, with the absolute build-host path
> +# stripped. This is essential in order to achieve reproducible builds.
> +#
> +# Typical usage:
> +#
> +#    inherit compiler-options
> +#    cc_extra=$(file_prefix_map_option_supported ${CC})
> +#    oe_runmake CFLAGS="${CFLAGS} $cc_extra"
> +#
> +
> +file_prefix_map_option_supported () {
> +	if $1 -Q --help=joined | grep -q "\-ffile-prefix-map=<old=new>"; then
> +		echo "-ffile-prefix-map=${S}=/${BP}/"
> +	fi
> +}
> 

Re: [PATCH 1/4] compiler-options.bbclass: provide and validate options

[Bystricky, Juro]

> On 12/13/17 3:31 PM, Juro Bystricky wrote:
> > As we may be using external toolchains, we cannot assume
> > various compiler options to be valid.
> >
> 
> While looking through my backlog :),  I think the idea is fine but
> implementation can be a bit more generic. e.g. a common function to
> probe the compiler for a given option then adding/removing it from
> compiler commandline.
> 

Yes, the code currently hardcodes test for --file-prefix-map and if it is supported, it uses/initializes it.
Not sure a generic API to query gcc command line options can be done easily though. Command line options are queried via various methods, i.e.:
${CC} -Q --help=target
${CC} -Q --help=optimizers
${CC} -Q --help=common
${CC} -Q --help=joined
...
Generally:

help={common|optimizers|params|target|warnings|[^]{joined|separate|undocumented}}[,...]

We could enumerate all help options, something like
${CC} -Q --help=target,common,optimizers,...

But that seems rather costly to do for each command line argument.
Of course, we could also pass the --help argument as an additional argument to the compiler options API to keep it a bit more efficient, but this could become ugly real quick.