All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/5] package/compiler-rt: new package
@ 2018-11-13 23:02 Matt Weber
  2018-11-13 23:02 ` [Buildroot] [PATCH 2/5] package/llvm: install target binary/debug tools Matt Weber
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Matt Weber @ 2018-11-13 23:02 UTC (permalink / raw)
  To: buildroot

This patch adds support for the compiler-rt (CLANG runtime) libary.
It builds a set of static libraries and installs them into the
CLANG/LLVM toolchain resource folder.

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
---
 DEVELOPERS                           |  1 +
 package/Config.in                    |  1 +
 package/compiler-rt/Config.in        | 12 ++++++++++++
 package/compiler-rt/compiler-rt.hash |  3 +++
 package/compiler-rt/compiler-rt.mk   | 35 +++++++++++++++++++++++++++++++++++
 5 files changed, 52 insertions(+)
 create mode 100644 package/compiler-rt/Config.in
 create mode 100644 package/compiler-rt/compiler-rt.hash
 create mode 100644 package/compiler-rt/compiler-rt.mk

diff --git a/DEVELOPERS b/DEVELOPERS
index 53467da..e78d649 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -1360,6 +1360,7 @@ F:	package/bridge-utils/
 F:	package/checkpolicy/
 F:	package/checksec/
 F:	package/cgroupfs-mount/
+F:	package/compiler-rt/
 F:	package/crda/
 F:	package/devmem2/
 F:	package/dnsmasq/
diff --git a/package/Config.in b/package/Config.in
index b60e770..73ddc2d 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -1602,6 +1602,7 @@ menu "Other"
 	source "package/clapack/Config.in"
 	source "package/classpath/Config.in"
 	source "package/cmocka/Config.in"
+	source "package/compiler-rt/Config.in"
 	source "package/cppcms/Config.in"
 	source "package/cracklib/Config.in"
 	source "package/dawgdic/Config.in"
diff --git a/package/compiler-rt/Config.in b/package/compiler-rt/Config.in
new file mode 100644
index 0000000..e15d3aa
--- /dev/null
+++ b/package/compiler-rt/Config.in
@@ -0,0 +1,12 @@
+config BR2_PACKAGE_COMPILER_RT
+	bool "compiler-rt"
+	depends on BR2_PACKAGE_LLVM
+	help
+	  A collection of runtime libraries primarily used by clang and
+	  llvm to provide builtins, sanitizer runtimes, and profiling
+	  at runtime.
+
+	  https://compiler-rt.llvm.org/
+
+comment "compiler-rt requires llvm to be enabled"
+	depends on !BR2_PACKAGE_LLVM
diff --git a/package/compiler-rt/compiler-rt.hash b/package/compiler-rt/compiler-rt.hash
new file mode 100644
index 0000000..df6ed19
--- /dev/null
+++ b/package/compiler-rt/compiler-rt.hash
@@ -0,0 +1,3 @@
+# Locally computed:
+sha256 bdec7fe3cf2c85f55656c07dfb0bd93ae46f2b3dd8f33ff3ad6e7586f4c670d6  compiler-rt-7.0.0.src.tar.xz
+sha256 417541d990edb3f96327ac03cb67e52eac80fc5c3e7afc69213cd04d7c3b9b27  LICENSE.TXT
diff --git a/package/compiler-rt/compiler-rt.mk b/package/compiler-rt/compiler-rt.mk
new file mode 100644
index 0000000..3f44639
--- /dev/null
+++ b/package/compiler-rt/compiler-rt.mk
@@ -0,0 +1,35 @@
+################################################################################
+#
+# compiler-rt
+#
+################################################################################
+
+# Compiler-RT should be bumped together with LLVM and Clang as the run-time is
+# tied to the version of those tools
+COMPILER_RT_VERSION = 7.0.0
+COMPILER_RT_SOURCE = compiler-rt-$(COMPILER_RT_VERSION).src.tar.xz
+COMPILER_RT_SITE = http://llvm.org/releases/$(COMPILER_RT_VERSION)
+COMPILER_RT_LICENSE = NCSA MIT
+COMPILER_RT_LICENSE_FILES = LICENSE.TXT
+COMPILER_RT_DEPENDENCIES = host-cmake host-clang llvm
+
+COMPILER_RT_INSTALL_STAGING = YES
+COMPILER_RT_INSTALL_TARGET = NO
+
+
+# -DCMAKE_INSTALL_PREFIX="" allows the COMPILER_RT_INSTALL_STAGING_CMDS to
+# provide the complete path to compilier-rt for installation of the runtime
+# libraries into the host-{clang,llvm} resources directory. The "resources"
+# directory is loosely document at this point and will probably need revisited
+# when making the llvm/clang tools SDK relocatable.
+COMPILER_RT_CONF_OPTS=-DCOMPILER_RT_INCLUDE_TESTS=ON \
+	-DCOMPILER_RT_STANDALONE_BUILD=ON \
+	-DCOMPILER_RT_DEFAULT_TARGET_TRIPLE=$(GNU_TARGET_NAME) \
+	-DLLVM_CONFIG_PATH=$(HOST_DIR)/usr/bin/llvm-config \
+	-DCMAKE_INSTALL_PREFIX=""
+
+define COMPILER_RT_INSTALL_STAGING_CMDS
+	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(HOST_DIR)/lib/clang/$(HOST_CLANG_VERSION)/ install/fast
+endef
+
+$(eval $(cmake-package))
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 2/5] package/llvm: install target binary/debug tools
  2018-11-13 23:02 [Buildroot] [PATCH 1/5] package/compiler-rt: new package Matt Weber
@ 2018-11-13 23:02 ` Matt Weber
  2018-11-15 17:05   ` Romain Naour
  2018-11-13 23:02 ` [Buildroot] [PATCH 3/5] support/testing/infra/emulator.py: support aarch64 Matt Weber
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Matt Weber @ 2018-11-13 23:02 UTC (permalink / raw)
  To: buildroot

The compiler-rt fuzzer and address sanitizer tools require additional
LLVM binary tools installed to allow stack trace decoding actively during
executable analysis.

https://github.com/google/sanitizers/wiki/AddressSanitizerCallStack

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
---
 package/llvm/llvm.mk | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/package/llvm/llvm.mk b/package/llvm/llvm.mk
index 1f9bd44..79477f0 100644
--- a/package/llvm/llvm.mk
+++ b/package/llvm/llvm.mk
@@ -197,8 +197,17 @@ HOST_LLVM_CONF_OPTS += \
 # We need to activate LLVM_INCLUDE_TOOLS, otherwise it does not generate
 # libLLVM.so
 LLVM_CONF_OPTS += \
-	-DLLVM_INCLUDE_TOOLS=ON \
+	-DLLVM_INCLUDE_TOOLS=ON
+
+# The llvm-symbolizer binary is used by the Compiler-RT Fuzzer
+# and AddressSanitizer tools for stack traces.
+ifeq ($(BR2_PACKAGE_COMPILER_RT),y)
+LLVM_CONF_OPTS += \
+	-DLLVM_BUILD_TOOLS=ON
+else
+LLVM_CONF_OPTS += \
 	-DLLVM_BUILD_TOOLS=OFF
+endif
 
 # Compiler-rt not in the source tree.
 # llvm runtime libraries are not in the source tree.
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 3/5] support/testing/infra/emulator.py: support aarch64
  2018-11-13 23:02 [Buildroot] [PATCH 1/5] package/compiler-rt: new package Matt Weber
  2018-11-13 23:02 ` [Buildroot] [PATCH 2/5] package/llvm: install target binary/debug tools Matt Weber
@ 2018-11-13 23:02 ` Matt Weber
  2018-11-16 22:13   ` Ricardo Martincoski
  2018-11-13 23:02 ` [Buildroot] [PATCH 4/5] support/testing/tests: CLANG compiler-rt runtime test Matt Weber
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Matt Weber @ 2018-11-13 23:02 UTC (permalink / raw)
  To: buildroot

 - Add the condition under the -kernel assignment for aarch64
 - Tested with a defconfig simliar to qemu_aarch64_virt_defconfig

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
---
 support/testing/infra/emulator.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
index 802e89d..8e5a7e9 100644
--- a/support/testing/infra/emulator.py
+++ b/support/testing/infra/emulator.py
@@ -62,7 +62,9 @@ class Emulator(object):
                     kernel = infra.download(self.downloaddir,
                                             "kernel-versatile")
                     qemu_cmd += ["-M", "versatilepb"]
-
+            elif arch == "aarch64":
+                kernel_cmdline.append("console=ttyAMA0")
+                qemu_cmd += ["-M", "virt", "-cpu", "cortex-a53"]
             qemu_cmd += ["-kernel", kernel]
 
         if kernel_cmdline:
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 4/5] support/testing/tests: CLANG compiler-rt runtime test
  2018-11-13 23:02 [Buildroot] [PATCH 1/5] package/compiler-rt: new package Matt Weber
  2018-11-13 23:02 ` [Buildroot] [PATCH 2/5] package/llvm: install target binary/debug tools Matt Weber
  2018-11-13 23:02 ` [Buildroot] [PATCH 3/5] support/testing/infra/emulator.py: support aarch64 Matt Weber
@ 2018-11-13 23:02 ` Matt Weber
  2018-11-16 23:45   ` Ricardo Martincoski
  2018-11-13 23:02 ` [Buildroot] [PATCH 5/5] llvm/clang: add note about version bumping together Matt Weber
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Matt Weber @ 2018-11-13 23:02 UTC (permalink / raw)
  To: buildroot

This patch adds a test case that
 1) Builds the complete LLVM and CLANG set of host tools
 2) Cross-compiles the compiler-rt runtime using CLANG
 3) Builds a cross-compiled application using CLANG and the libfuzzer
    compiler-rt library.
 4) Executes the fuzz application on target and checkes expected output

Credit to the fuzzer test suite for the tutorial example used as the
fuzz test application.
https://github.com/google/fuzzer-test-suite

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
---
 support/testing/tests/package/test_clang.py | 93 +++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 support/testing/tests/package/test_clang.py

diff --git a/support/testing/tests/package/test_clang.py b/support/testing/tests/package/test_clang.py
new file mode 100644
index 0000000..9c42c0d
--- /dev/null
+++ b/support/testing/tests/package/test_clang.py
@@ -0,0 +1,93 @@
+import os
+import tempfile
+import subprocess
+import shutil
+
+import infra.basetest
+
+FUZZ_TIMEOUT = 120
+
+
+class TestClangBase(infra.basetest.BRTest):
+
+    def login(self):
+        img = os.path.join(self.builddir, "images", "rootfs.cpio.gz")
+        kern = os.path.join(self.builddir, "images", "Image")
+        # Sanitizers overallocate memory and the minimum that seemed to work was 512MB
+        self.emulator.boot(arch="aarch64",
+                           kernel=kern,
+                           options=["-m", "512", "-initrd", img])
+        self.emulator.login()
+
+    def build_test_prog(self):
+        hostdir = os.path.join(self.builddir, 'host')
+        linkerdir = os.path.join(hostdir, 'opt', 'ext-toolchain')
+        stagingdir = os.path.join(self.builddir, 'staging')
+        env = os.environ.copy()
+        env["PATH"] = "{}:".format(os.path.join(hostdir, 'bin')) + env["PATH"]
+        clangcpp = os.path.join(hostdir, 'bin', 'clang++')
+        workdir = os.path.join(tempfile.mkdtemp(suffix='-br2-testing-compiler-rt'))
+        fuzz_src = os.path.join(workdir, 'fuzz_me.cc')
+        fuzz_bin = os.path.join(workdir, 'fuzz_me')
+        with open(fuzz_src, 'w+') as source_file:
+            source_file.write('#include <stdint.h>\n')
+            source_file.write('#include <stddef.h>\n')
+            source_file.write('bool FuzzMe(const uint8_t *Data, size_t DataSize) {\n')
+            source_file.write('  return DataSize >= 3 &&\n')
+            source_file.write('      Data[0] == \'F\' &&\n')
+            source_file.write('      Data[1] == \'U\' &&\n')
+            source_file.write('      Data[2] == \'Z\' &&\n')
+            source_file.write('      Data[3] == \'Z\';\n')
+            source_file.write('}\n')
+            source_file.write('extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {\n')
+            source_file.write('  FuzzMe(Data, Size);\n')
+            source_file.write('  return 0;\n')
+            source_file.write('}\n')
+
+        cmd = [clangcpp,
+               '-mcpu=cortex-a53',
+               '--sysroot', stagingdir,
+               '-B', linkerdir,
+               '-fsanitize=address,fuzzer',
+               fuzz_src,
+               '-o', fuzz_bin]
+        ret = subprocess.call(cmd,
+                              stdout=self.b.logfile,
+                              stderr=self.b.logfile,
+                              env=env)
+        if ret != 0:
+            raise SystemError("Clang build process launch failed")
+
+        shutil.copy(fuzz_bin, os.path.join(self.builddir, 'target', 'usr', 'bin'))
+        self.b.build()
+        shutil.rmtree(workdir)
+
+
+class TestClangCompilerRT(TestClangBase):
+    config = \
+             """
+             BR2_aarch64=y
+             BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
+             BR2_TOOLCHAIN_EXTERNAL=y
+             BR2_LINUX_KERNEL=y
+             BR2_LINUX_KERNEL_CUSTOM_VERSION=y
+             BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.16.7"
+             BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y
+             BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="board/qemu/aarch64-virt/linux.config"
+             BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
+             BR2_TARGET_ROOTFS_CPIO=y
+             BR2_TARGET_ROOTFS_CPIO_GZIP=y
+             # BR2_TARGET_ROOTFS_TAR is not set
+             BR2_PACKAGE_COMPILER_RT=y
+             BR2_PACKAGE_LLVM=y
+             """
+
+    def test_run(self):
+        self.build_test_prog()
+        self.login()
+
+        # The test case verifies both that the application executes and that
+        # the symbolizer is working to decode the stack trace
+        cmd = "fuzz_me 2>&1 | grep _M_replace"
+        _, exit_code = self.emulator.run(cmd, FUZZ_TIMEOUT)
+        self.assertEqual(exit_code, 0)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 5/5] llvm/clang: add note about version bumping together
  2018-11-13 23:02 [Buildroot] [PATCH 1/5] package/compiler-rt: new package Matt Weber
                   ` (2 preceding siblings ...)
  2018-11-13 23:02 ` [Buildroot] [PATCH 4/5] support/testing/tests: CLANG compiler-rt runtime test Matt Weber
@ 2018-11-13 23:02 ` Matt Weber
  2018-11-15 16:32   ` Romain Naour
  2018-11-14  3:07 ` [Buildroot] [PATCH 1/5] package/compiler-rt: new package Matthew Weber
  2018-11-15 16:46 ` Romain Naour
  5 siblings, 1 reply; 20+ messages in thread
From: Matt Weber @ 2018-11-13 23:02 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
---
 package/clang/clang.mk | 1 +
 package/llvm/llvm.mk   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/package/clang/clang.mk b/package/clang/clang.mk
index f9d4ae3..16f93ee 100644
--- a/package/clang/clang.mk
+++ b/package/clang/clang.mk
@@ -4,6 +4,7 @@
 #
 ################################################################################
 
+# LLVM and Clang should be version bumped together
 CLANG_VERSION = 7.0.0
 CLANG_SITE = http://llvm.org/releases/$(CLANG_VERSION)
 CLANG_SOURCE = cfe-$(CLANG_VERSION).src.tar.xz
diff --git a/package/llvm/llvm.mk b/package/llvm/llvm.mk
index 79477f0..56f6ed9 100644
--- a/package/llvm/llvm.mk
+++ b/package/llvm/llvm.mk
@@ -4,6 +4,7 @@
 #
 ################################################################################
 
+# LLVM and Clang should be version bumped together
 LLVM_VERSION = 7.0.0
 LLVM_SITE = http://llvm.org/releases/$(LLVM_VERSION)
 LLVM_SOURCE = llvm-$(LLVM_VERSION).src.tar.xz
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/5] package/compiler-rt: new package
  2018-11-13 23:02 [Buildroot] [PATCH 1/5] package/compiler-rt: new package Matt Weber
                   ` (3 preceding siblings ...)
  2018-11-13 23:02 ` [Buildroot] [PATCH 5/5] llvm/clang: add note about version bumping together Matt Weber
@ 2018-11-14  3:07 ` Matthew Weber
  2018-11-15 16:46 ` Romain Naour
  5 siblings, 0 replies; 20+ messages in thread
From: Matthew Weber @ 2018-11-14  3:07 UTC (permalink / raw)
  To: buildroot

+ Romain / Valentin
On Tue, Nov 13, 2018 at 5:02 PM Matt Weber
<matthew.weber@rockwellcollins.com> wrote:
>
> This patch adds support for the compiler-rt (CLANG runtime) libary.
> It builds a set of static libraries and installs them into the
> CLANG/LLVM toolchain resource folder.
>
> Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> ---
>  DEVELOPERS                           |  1 +
>  package/Config.in                    |  1 +
>  package/compiler-rt/Config.in        | 12 ++++++++++++
>  package/compiler-rt/compiler-rt.hash |  3 +++
>  package/compiler-rt/compiler-rt.mk   | 35 +++++++++++++++++++++++++++++++++++
>  5 files changed, 52 insertions(+)
>  create mode 100644 package/compiler-rt/Config.in
>  create mode 100644 package/compiler-rt/compiler-rt.hash
>  create mode 100644 package/compiler-rt/compiler-rt.mk
>
> diff --git a/DEVELOPERS b/DEVELOPERS
> index 53467da..e78d649 100644
> --- a/DEVELOPERS
> +++ b/DEVELOPERS
> @@ -1360,6 +1360,7 @@ F:        package/bridge-utils/
>  F:     package/checkpolicy/
>  F:     package/checksec/
>  F:     package/cgroupfs-mount/
> +F:     package/compiler-rt/
>  F:     package/crda/
>  F:     package/devmem2/
>  F:     package/dnsmasq/
> diff --git a/package/Config.in b/package/Config.in
> index b60e770..73ddc2d 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -1602,6 +1602,7 @@ menu "Other"
>         source "package/clapack/Config.in"
>         source "package/classpath/Config.in"
>         source "package/cmocka/Config.in"
> +       source "package/compiler-rt/Config.in"
>         source "package/cppcms/Config.in"
>         source "package/cracklib/Config.in"
>         source "package/dawgdic/Config.in"
> diff --git a/package/compiler-rt/Config.in b/package/compiler-rt/Config.in
> new file mode 100644
> index 0000000..e15d3aa
> --- /dev/null
> +++ b/package/compiler-rt/Config.in
> @@ -0,0 +1,12 @@
> +config BR2_PACKAGE_COMPILER_RT
> +       bool "compiler-rt"
> +       depends on BR2_PACKAGE_LLVM
> +       help
> +         A collection of runtime libraries primarily used by clang and
> +         llvm to provide builtins, sanitizer runtimes, and profiling
> +         at runtime.
> +
> +         https://compiler-rt.llvm.org/
> +
> +comment "compiler-rt requires llvm to be enabled"
> +       depends on !BR2_PACKAGE_LLVM
> diff --git a/package/compiler-rt/compiler-rt.hash b/package/compiler-rt/compiler-rt.hash
> new file mode 100644
> index 0000000..df6ed19
> --- /dev/null
> +++ b/package/compiler-rt/compiler-rt.hash
> @@ -0,0 +1,3 @@
> +# Locally computed:
> +sha256 bdec7fe3cf2c85f55656c07dfb0bd93ae46f2b3dd8f33ff3ad6e7586f4c670d6  compiler-rt-7.0.0.src.tar.xz
> +sha256 417541d990edb3f96327ac03cb67e52eac80fc5c3e7afc69213cd04d7c3b9b27  LICENSE.TXT
> diff --git a/package/compiler-rt/compiler-rt.mk b/package/compiler-rt/compiler-rt.mk
> new file mode 100644
> index 0000000..3f44639
> --- /dev/null
> +++ b/package/compiler-rt/compiler-rt.mk
> @@ -0,0 +1,35 @@
> +################################################################################
> +#
> +# compiler-rt
> +#
> +################################################################################
> +
> +# Compiler-RT should be bumped together with LLVM and Clang as the run-time is
> +# tied to the version of those tools
> +COMPILER_RT_VERSION = 7.0.0
> +COMPILER_RT_SOURCE = compiler-rt-$(COMPILER_RT_VERSION).src.tar.xz
> +COMPILER_RT_SITE = http://llvm.org/releases/$(COMPILER_RT_VERSION)
> +COMPILER_RT_LICENSE = NCSA MIT
> +COMPILER_RT_LICENSE_FILES = LICENSE.TXT
> +COMPILER_RT_DEPENDENCIES = host-cmake host-clang llvm
> +
> +COMPILER_RT_INSTALL_STAGING = YES
> +COMPILER_RT_INSTALL_TARGET = NO
> +
> +
> +# -DCMAKE_INSTALL_PREFIX="" allows the COMPILER_RT_INSTALL_STAGING_CMDS to
> +# provide the complete path to compilier-rt for installation of the runtime
> +# libraries into the host-{clang,llvm} resources directory. The "resources"
> +# directory is loosely document at this point and will probably need revisited
> +# when making the llvm/clang tools SDK relocatable.
> +COMPILER_RT_CONF_OPTS=-DCOMPILER_RT_INCLUDE_TESTS=ON \
> +       -DCOMPILER_RT_STANDALONE_BUILD=ON \
> +       -DCOMPILER_RT_DEFAULT_TARGET_TRIPLE=$(GNU_TARGET_NAME) \
> +       -DLLVM_CONFIG_PATH=$(HOST_DIR)/usr/bin/llvm-config \
> +       -DCMAKE_INSTALL_PREFIX=""
> +
> +define COMPILER_RT_INSTALL_STAGING_CMDS
> +       $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(HOST_DIR)/lib/clang/$(HOST_CLANG_VERSION)/ install/fast
> +endef
> +
> +$(eval $(cmake-package))
> --
> 1.9.1
>


-- 
Matthew L Weber / Pr Software Engineer
Airborne Information Systems / RC Linux Secure Platforms
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
www.rockwellcollins.com

Note: Any Export License Required Information and License Restricted
Third Party Intellectual Property (TPIP) content must be encrypted and
sent to matthew.weber at corp.rockwellcollins.com.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 5/5] llvm/clang: add note about version bumping together
  2018-11-13 23:02 ` [Buildroot] [PATCH 5/5] llvm/clang: add note about version bumping together Matt Weber
@ 2018-11-15 16:32   ` Romain Naour
  0 siblings, 0 replies; 20+ messages in thread
From: Romain Naour @ 2018-11-15 16:32 UTC (permalink / raw)
  To: buildroot

Hi Matt,

Le 14/11/2018 ? 00:02, Matt Weber a ?crit?:
> Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>

This patch is not related to compiler-rt and can me merged as is.

Acked-by: Romain Naour <romain.naour@smile.fr>

Best regards,
Romain

> ---
>  package/clang/clang.mk | 1 +
>  package/llvm/llvm.mk   | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/package/clang/clang.mk b/package/clang/clang.mk
> index f9d4ae3..16f93ee 100644
> --- a/package/clang/clang.mk
> +++ b/package/clang/clang.mk
> @@ -4,6 +4,7 @@
>  #
>  ################################################################################
>  
> +# LLVM and Clang should be version bumped together
>  CLANG_VERSION = 7.0.0
>  CLANG_SITE = http://llvm.org/releases/$(CLANG_VERSION)
>  CLANG_SOURCE = cfe-$(CLANG_VERSION).src.tar.xz
> diff --git a/package/llvm/llvm.mk b/package/llvm/llvm.mk
> index 79477f0..56f6ed9 100644
> --- a/package/llvm/llvm.mk
> +++ b/package/llvm/llvm.mk
> @@ -4,6 +4,7 @@
>  #
>  ################################################################################
>  
> +# LLVM and Clang should be version bumped together
>  LLVM_VERSION = 7.0.0
>  LLVM_SITE = http://llvm.org/releases/$(LLVM_VERSION)
>  LLVM_SOURCE = llvm-$(LLVM_VERSION).src.tar.xz
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/5] package/compiler-rt: new package
  2018-11-13 23:02 [Buildroot] [PATCH 1/5] package/compiler-rt: new package Matt Weber
                   ` (4 preceding siblings ...)
  2018-11-14  3:07 ` [Buildroot] [PATCH 1/5] package/compiler-rt: new package Matthew Weber
@ 2018-11-15 16:46 ` Romain Naour
  2018-11-15 20:41   ` Matthew Weber
  5 siblings, 1 reply; 20+ messages in thread
From: Romain Naour @ 2018-11-15 16:46 UTC (permalink / raw)
  To: buildroot

Hi Matt,

Le 14/11/2018 ? 00:02, Matt Weber a ?crit?:
> This patch adds support for the compiler-rt (CLANG runtime) libary.
> It builds a set of static libraries and installs them into the
> CLANG/LLVM toolchain resource folder.
> 
> Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> ---
>  DEVELOPERS                           |  1 +
>  package/Config.in                    |  1 +
>  package/compiler-rt/Config.in        | 12 ++++++++++++
>  package/compiler-rt/compiler-rt.hash |  3 +++
>  package/compiler-rt/compiler-rt.mk   | 35 +++++++++++++++++++++++++++++++++++
>  5 files changed, 52 insertions(+)
>  create mode 100644 package/compiler-rt/Config.in
>  create mode 100644 package/compiler-rt/compiler-rt.hash
>  create mode 100644 package/compiler-rt/compiler-rt.mk
> 
> diff --git a/DEVELOPERS b/DEVELOPERS
> index 53467da..e78d649 100644
> --- a/DEVELOPERS
> +++ b/DEVELOPERS
> @@ -1360,6 +1360,7 @@ F:	package/bridge-utils/
>  F:	package/checkpolicy/
>  F:	package/checksec/
>  F:	package/cgroupfs-mount/
> +F:	package/compiler-rt/
>  F:	package/crda/
>  F:	package/devmem2/
>  F:	package/dnsmasq/
> diff --git a/package/Config.in b/package/Config.in
> index b60e770..73ddc2d 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -1602,6 +1602,7 @@ menu "Other"
>  	source "package/clapack/Config.in"
>  	source "package/classpath/Config.in"
>  	source "package/cmocka/Config.in"
> +	source "package/compiler-rt/Config.in"
>  	source "package/cppcms/Config.in"
>  	source "package/cracklib/Config.in"
>  	source "package/dawgdic/Config.in"
> diff --git a/package/compiler-rt/Config.in b/package/compiler-rt/Config.in
> new file mode 100644
> index 0000000..e15d3aa
> --- /dev/null
> +++ b/package/compiler-rt/Config.in
> @@ -0,0 +1,12 @@
> +config BR2_PACKAGE_COMPILER_RT
> +	bool "compiler-rt"
> +	depends on BR2_PACKAGE_LLVM
> +	help
> +	  A collection of runtime libraries primarily used by clang and
> +	  llvm to provide builtins, sanitizer runtimes, and profiling
> +	  at runtime.
> +
> +	  https://compiler-rt.llvm.org/
> +
> +comment "compiler-rt requires llvm to be enabled"
> +	depends on !BR2_PACKAGE_LLVM
> diff --git a/package/compiler-rt/compiler-rt.hash b/package/compiler-rt/compiler-rt.hash
> new file mode 100644
> index 0000000..df6ed19
> --- /dev/null
> +++ b/package/compiler-rt/compiler-rt.hash
> @@ -0,0 +1,3 @@
> +# Locally computed:
> +sha256 bdec7fe3cf2c85f55656c07dfb0bd93ae46f2b3dd8f33ff3ad6e7586f4c670d6  compiler-rt-7.0.0.src.tar.xz
> +sha256 417541d990edb3f96327ac03cb67e52eac80fc5c3e7afc69213cd04d7c3b9b27  LICENSE.TXT
> diff --git a/package/compiler-rt/compiler-rt.mk b/package/compiler-rt/compiler-rt.mk
> new file mode 100644
> index 0000000..3f44639
> --- /dev/null
> +++ b/package/compiler-rt/compiler-rt.mk
> @@ -0,0 +1,35 @@
> +################################################################################
> +#
> +# compiler-rt
> +#
> +################################################################################
> +
> +# Compiler-RT should be bumped together with LLVM and Clang as the run-time is
> +# tied to the version of those tools
> +COMPILER_RT_VERSION = 7.0.0
> +COMPILER_RT_SOURCE = compiler-rt-$(COMPILER_RT_VERSION).src.tar.xz
> +COMPILER_RT_SITE = http://llvm.org/releases/$(COMPILER_RT_VERSION)
> +COMPILER_RT_LICENSE = NCSA MIT
> +COMPILER_RT_LICENSE_FILES = LICENSE.TXT
> +COMPILER_RT_DEPENDENCIES = host-cmake host-clang llvm

No need to add host-cmake since compiler-rt is a cmake based package.

> +
> +COMPILER_RT_INSTALL_STAGING = YES
> +COMPILER_RT_INSTALL_TARGET = NO

Compiler-rt is a "collection of runtime libraries", so it's weird to not install
them on the target. Maybe it's due to compiler-rt provide only static libraries?
In that case it's fine but can you add a comment?

> +
> +
> +# -DCMAKE_INSTALL_PREFIX="" allows the COMPILER_RT_INSTALL_STAGING_CMDS to
> +# provide the complete path to compilier-rt for installation of the runtime
> +# libraries into the host-{clang,llvm} resources directory. The "resources"
> +# directory is loosely document at this point and will probably need revisited
> +# when making the llvm/clang tools SDK relocatable.
> +COMPILER_RT_CONF_OPTS=-DCOMPILER_RT_INCLUDE_TESTS=ON \
> +	-DCOMPILER_RT_STANDALONE_BUILD=ON \
> +	-DCOMPILER_RT_DEFAULT_TARGET_TRIPLE=$(GNU_TARGET_NAME) \
> +	-DLLVM_CONFIG_PATH=$(HOST_DIR)/usr/bin/llvm-config \
> +	-DCMAKE_INSTALL_PREFIX=""
> +
> +define COMPILER_RT_INSTALL_STAGING_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(HOST_DIR)/lib/clang/$(HOST_CLANG_VERSION)/ install/fast
> +endef

I need to take a look, usually CMAKE_INSTALL_PREFIX should be set to /usr.

Best regards,
Romain

> +
> +$(eval $(cmake-package))
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 2/5] package/llvm: install target binary/debug tools
  2018-11-13 23:02 ` [Buildroot] [PATCH 2/5] package/llvm: install target binary/debug tools Matt Weber
@ 2018-11-15 17:05   ` Romain Naour
  0 siblings, 0 replies; 20+ messages in thread
From: Romain Naour @ 2018-11-15 17:05 UTC (permalink / raw)
  To: buildroot

Hi Matt,

Le 14/11/2018 ? 00:02, Matt Weber a ?crit?:
> The compiler-rt fuzzer and address sanitizer tools require additional
> LLVM binary tools installed to allow stack trace decoding actively during
> executable analysis.
> 
> https://github.com/google/sanitizers/wiki/AddressSanitizerCallStack
> 
> Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> ---
>  package/llvm/llvm.mk | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/package/llvm/llvm.mk b/package/llvm/llvm.mk
> index 1f9bd44..79477f0 100644
> --- a/package/llvm/llvm.mk
> +++ b/package/llvm/llvm.mk
> @@ -197,8 +197,17 @@ HOST_LLVM_CONF_OPTS += \
>  # We need to activate LLVM_INCLUDE_TOOLS, otherwise it does not generate
>  # libLLVM.so
>  LLVM_CONF_OPTS += \
> -	-DLLVM_INCLUDE_TOOLS=ON \
> +	-DLLVM_INCLUDE_TOOLS=ON
> +
> +# The llvm-symbolizer binary is used by the Compiler-RT Fuzzer
> +# and AddressSanitizer tools for stack traces.
> +ifeq ($(BR2_PACKAGE_COMPILER_RT),y)
> +LLVM_CONF_OPTS += \
> +	-DLLVM_BUILD_TOOLS=ON
> +else
> +LLVM_CONF_OPTS += \
>  	-DLLVM_BUILD_TOOLS=OFF
> +endif

Reviewed-by: Romain Naour <romain.naour@smile.fr>

Best regards,
Romain

>  
>  # Compiler-rt not in the source tree.
>  # llvm runtime libraries are not in the source tree.
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 1/5] package/compiler-rt: new package
  2018-11-15 16:46 ` Romain Naour
@ 2018-11-15 20:41   ` Matthew Weber
  0 siblings, 0 replies; 20+ messages in thread
From: Matthew Weber @ 2018-11-15 20:41 UTC (permalink / raw)
  To: buildroot

Romain,


On Thu, Nov 15, 2018 at 10:46 AM Romain Naour <romain.naour@smile.fr> wrote:
>
> Hi Matt,
>
> Le 14/11/2018 ? 00:02, Matt Weber a ?crit :
> > This patch adds support for the compiler-rt (CLANG runtime) libary.
> > It builds a set of static libraries and installs them into the
> > CLANG/LLVM toolchain resource folder.
> >
> > Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> > ---
> >  DEVELOPERS                           |  1 +
> >  package/Config.in                    |  1 +
> >  package/compiler-rt/Config.in        | 12 ++++++++++++
> >  package/compiler-rt/compiler-rt.hash |  3 +++
> >  package/compiler-rt/compiler-rt.mk   | 35 +++++++++++++++++++++++++++++++++++
> >  5 files changed, 52 insertions(+)
> >  create mode 100644 package/compiler-rt/Config.in
> >  create mode 100644 package/compiler-rt/compiler-rt.hash
> >  create mode 100644 package/compiler-rt/compiler-rt.mk
> >
> > diff --git a/DEVELOPERS b/DEVELOPERS
> > index 53467da..e78d649 100644
> > --- a/DEVELOPERS
> > +++ b/DEVELOPERS
> > @@ -1360,6 +1360,7 @@ F:      package/bridge-utils/
> >  F:   package/checkpolicy/
> >  F:   package/checksec/
> >  F:   package/cgroupfs-mount/
> > +F:   package/compiler-rt/
> >  F:   package/crda/
> >  F:   package/devmem2/
> >  F:   package/dnsmasq/
> > diff --git a/package/Config.in b/package/Config.in
> > index b60e770..73ddc2d 100644
> > --- a/package/Config.in
> > +++ b/package/Config.in
> > @@ -1602,6 +1602,7 @@ menu "Other"
> >       source "package/clapack/Config.in"
> >       source "package/classpath/Config.in"
> >       source "package/cmocka/Config.in"
> > +     source "package/compiler-rt/Config.in"
> >       source "package/cppcms/Config.in"
> >       source "package/cracklib/Config.in"
> >       source "package/dawgdic/Config.in"
> > diff --git a/package/compiler-rt/Config.in b/package/compiler-rt/Config.in
> > new file mode 100644
> > index 0000000..e15d3aa
> > --- /dev/null
> > +++ b/package/compiler-rt/Config.in
> > @@ -0,0 +1,12 @@
> > +config BR2_PACKAGE_COMPILER_RT
> > +     bool "compiler-rt"
> > +     depends on BR2_PACKAGE_LLVM
> > +     help
> > +       A collection of runtime libraries primarily used by clang and
> > +       llvm to provide builtins, sanitizer runtimes, and profiling
> > +       at runtime.
> > +
> > +       https://compiler-rt.llvm.org/
> > +
> > +comment "compiler-rt requires llvm to be enabled"
> > +     depends on !BR2_PACKAGE_LLVM
> > diff --git a/package/compiler-rt/compiler-rt.hash b/package/compiler-rt/compiler-rt.hash
> > new file mode 100644
> > index 0000000..df6ed19
> > --- /dev/null
> > +++ b/package/compiler-rt/compiler-rt.hash
> > @@ -0,0 +1,3 @@
> > +# Locally computed:
> > +sha256 bdec7fe3cf2c85f55656c07dfb0bd93ae46f2b3dd8f33ff3ad6e7586f4c670d6  compiler-rt-7.0.0.src.tar.xz
> > +sha256 417541d990edb3f96327ac03cb67e52eac80fc5c3e7afc69213cd04d7c3b9b27  LICENSE.TXT
> > diff --git a/package/compiler-rt/compiler-rt.mk b/package/compiler-rt/compiler-rt.mk
> > new file mode 100644
> > index 0000000..3f44639
> > --- /dev/null
> > +++ b/package/compiler-rt/compiler-rt.mk
> > @@ -0,0 +1,35 @@
> > +################################################################################
> > +#
> > +# compiler-rt
> > +#
> > +################################################################################
> > +
> > +# Compiler-RT should be bumped together with LLVM and Clang as the run-time is
> > +# tied to the version of those tools
> > +COMPILER_RT_VERSION = 7.0.0
> > +COMPILER_RT_SOURCE = compiler-rt-$(COMPILER_RT_VERSION).src.tar.xz
> > +COMPILER_RT_SITE = http://llvm.org/releases/$(COMPILER_RT_VERSION)
> > +COMPILER_RT_LICENSE = NCSA MIT
> > +COMPILER_RT_LICENSE_FILES = LICENSE.TXT
> > +COMPILER_RT_DEPENDENCIES = host-cmake host-clang llvm
>
> No need to add host-cmake since compiler-rt is a cmake based package.

True, funny what you miss when you review your own work :-) I'll remove that.

>
> > +
> > +COMPILER_RT_INSTALL_STAGING = YES
> > +COMPILER_RT_INSTALL_TARGET = NO
>
> Compiler-rt is a "collection of runtime libraries", so it's weird to not install
> them on the target. Maybe it's due to compiler-rt provide only static libraries?
> In that case it's fine but can you add a comment?

For this initial usecase, yes the libraries I'm using are just static.
Maybe a comment like follows?

"Only static libraries are currently assumed to support a libfuzzer
use-case, however there could be use-cases to install shared
libraries, if compiler-rt is used in other forms to build executables
with Clang."

>
> > +
> > +
> > +# -DCMAKE_INSTALL_PREFIX="" allows the COMPILER_RT_INSTALL_STAGING_CMDS to
> > +# provide the complete path to compilier-rt for installation of the runtime
> > +# libraries into the host-{clang,llvm} resources directory. The "resources"
> > +# directory is loosely document at this point and will probably need revisited
> > +# when making the llvm/clang tools SDK relocatable.
> > +COMPILER_RT_CONF_OPTS=-DCOMPILER_RT_INCLUDE_TESTS=ON \
> > +     -DCOMPILER_RT_STANDALONE_BUILD=ON \
> > +     -DCOMPILER_RT_DEFAULT_TARGET_TRIPLE=$(GNU_TARGET_NAME) \
> > +     -DLLVM_CONFIG_PATH=$(HOST_DIR)/usr/bin/llvm-config \
> > +     -DCMAKE_INSTALL_PREFIX=""
> > +
> > +define COMPILER_RT_INSTALL_STAGING_CMDS
> > +     $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) DESTDIR=$(HOST_DIR)/lib/clang/$(HOST_CLANG_VERSION)/ install/fast
> > +endef
>
> I need to take a look, usually CMAKE_INSTALL_PREFIX should be set to /usr.

Agree, however currently I end up with a pathing problem I haven't
been able to solve yet.
 i.e. if CMAKE_INSTALL_PREFIX=/usr
/home/foobar/buildroot/output/host/lib/clang/7.0.0/usr/lib/*.a

 i.e. if CMAKE_INSTALL_PREFIX=""
/home/foobar/buildroot/output/host/lib/clang/7.0.0/lib/*.a

The toolchain is actually looking in the location
CMAKE_INSTALL_PREFIX="" for the runtime libraries it uses to build.

Thank you for the review on this series,
Matt

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 3/5] support/testing/infra/emulator.py: support aarch64
  2018-11-13 23:02 ` [Buildroot] [PATCH 3/5] support/testing/infra/emulator.py: support aarch64 Matt Weber
@ 2018-11-16 22:13   ` Ricardo Martincoski
  2018-11-17  0:57     ` Matthew Weber
  0 siblings, 1 reply; 20+ messages in thread
From: Ricardo Martincoski @ 2018-11-16 22:13 UTC (permalink / raw)
  To: buildroot

Hello,

+ Thomas

On Tue, Nov 13, 2018 at 09:02 PM, Matt Weber wrote:

>  - Add the condition under the -kernel assignment for aarch64
>  - Tested with a defconfig simliar to qemu_aarch64_virt_defconfig

s/simliar/similar

> 
> Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> ---
>  support/testing/infra/emulator.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
> index 802e89d..8e5a7e9 100644
> --- a/support/testing/infra/emulator.py
> +++ b/support/testing/infra/emulator.py
> @@ -62,7 +62,9 @@ class Emulator(object):
>                      kernel = infra.download(self.downloaddir,
>                                              "kernel-versatile")
>                      qemu_cmd += ["-M", "versatilepb"]
> -
> +            elif arch == "aarch64":
> +                kernel_cmdline.append("console=ttyAMA0")
> +                qemu_cmd += ["-M", "virt", "-cpu", "cortex-a53"]

We don't really need this code here.
You could do this in the test case:

self.emulator.boot(arch="aarch64",
                   kernel=kern,
                   kernel_cmdline=["console=ttyAMA0"],
                   options=["-M", "virt", "-cpu", "cortex-a53", "-m", "512", "-initrd", img])

If we think more and more test cases would need/want this we could add this
here, preferably as a "builtin" pre-compiled kernel for aarch64.

But for now, I suggest to move this code to the test case.


Regards,
Ricardo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 4/5] support/testing/tests: CLANG compiler-rt runtime test
  2018-11-13 23:02 ` [Buildroot] [PATCH 4/5] support/testing/tests: CLANG compiler-rt runtime test Matt Weber
@ 2018-11-16 23:45   ` Ricardo Martincoski
  2018-11-17  1:15     ` Matthew Weber
  0 siblings, 1 reply; 20+ messages in thread
From: Ricardo Martincoski @ 2018-11-16 23:45 UTC (permalink / raw)
  To: buildroot

Hello,

In overall it looks good.

On Tue, Nov 13, 2018 at 09:02 PM, Matt Weber wrote:

> This patch adds a test case that
>  1) Builds the complete LLVM and CLANG set of host tools
>  2) Cross-compiles the compiler-rt runtime using CLANG
>  3) Builds a cross-compiled application using CLANG and the libfuzzer
>     compiler-rt library.
>  4) Executes the fuzz application on target and checkes expected output
> 
> Credit to the fuzzer test suite for the tutorial example used as the
> fuzz test application.
> https://github.com/google/fuzzer-test-suite
> 
> Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> ---
>  support/testing/tests/package/test_clang.py | 93 +++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 support/testing/tests/package/test_clang.py

The change to .gitlab-ci.yml (resulting from 'make .gitlab-ci.yml') is missing.

> 
> diff --git a/support/testing/tests/package/test_clang.py b/support/testing/tests/package/test_clang.py
> new file mode 100644
> index 0000000..9c42c0d
> --- /dev/null
> +++ b/support/testing/tests/package/test_clang.py
> @@ -0,0 +1,93 @@
> +import os
> +import tempfile
> +import subprocess
> +import shutil
> +
> +import infra.basetest
> +
> +FUZZ_TIMEOUT = 120
> +
> +
> +class TestClangBase(infra.basetest.BRTest):
> +
> +    def login(self):
> +        img = os.path.join(self.builddir, "images", "rootfs.cpio.gz")
> +        kern = os.path.join(self.builddir, "images", "Image")
> +        # Sanitizers overallocate memory and the minimum that seemed to work was 512MB
> +        self.emulator.boot(arch="aarch64",
> +                           kernel=kern,
> +                           options=["-m", "512", "-initrd", img])

Here you could merge in the contents of patch 3, as I replied there.

> +        self.emulator.login()
> +
> +    def build_test_prog(self):
> +        hostdir = os.path.join(self.builddir, 'host')
> +        linkerdir = os.path.join(hostdir, 'opt', 'ext-toolchain')
> +        stagingdir = os.path.join(self.builddir, 'staging')
> +        env = os.environ.copy()
> +        env["PATH"] = "{}:".format(os.path.join(hostdir, 'bin')) + env["PATH"]
> +        clangcpp = os.path.join(hostdir, 'bin', 'clang++')
> +        workdir = os.path.join(tempfile.mkdtemp(suffix='-br2-testing-compiler-rt'))
> +        fuzz_src = os.path.join(workdir, 'fuzz_me.cc')
> +        fuzz_bin = os.path.join(workdir, 'fuzz_me')
> +        with open(fuzz_src, 'w+') as source_file:
> +            source_file.write('#include <stdint.h>\n')
> +            source_file.write('#include <stddef.h>\n')
> +            source_file.write('bool FuzzMe(const uint8_t *Data, size_t DataSize) {\n')
> +            source_file.write('  return DataSize >= 3 &&\n')
> +            source_file.write('      Data[0] == \'F\' &&\n')
> +            source_file.write('      Data[1] == \'U\' &&\n')
> +            source_file.write('      Data[2] == \'Z\' &&\n')
> +            source_file.write('      Data[3] == \'Z\';\n')
> +            source_file.write('}\n')
> +            source_file.write('extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {\n')
> +            source_file.write('  FuzzMe(Data, Size);\n')
> +            source_file.write('  return 0;\n')
> +            source_file.write('}\n')

I dislike this a bit for 2 reasons:

1) The contents of the file would be much more readable in a separate file. See
   for example the changes introduced by commit "ee6b37cf87 support/testing: use
   TestPythonPackageBase for python-incremental";

2) This is a package for the target. The most correct way of handling this IMO
   is to use a package in a br2-external.
   Unfortunately this support is not yet merged to the test infra.
   If you like the idea, you could pull and resend 2 patches of mine: [1] [2]
   Here is a sketch, poorly tested because... well... this test case takes a lot
   of time to run: [3]
   And if you follow this road, you could even move the login() method and use a
   single class: [4]

What do you think about this?

[snip]
> +class TestClangCompilerRT(TestClangBase):
> +    config = \
> +             """

I see that you used test_rust as base.
This test (rust) is the last remaining using a defconfig fragment with
non-standard style.
I just sent a patch to fix it: [5]
Could do the same here?

> +             BR2_aarch64=y

> +             BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
> +             BR2_TOOLCHAIN_EXTERNAL=y

nit: this is not the order after 'make savedefconfig'

> +             BR2_LINUX_KERNEL=y
> +             BR2_LINUX_KERNEL_CUSTOM_VERSION=y
> +             BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.16.7"
> +             BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y
> +             BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="board/qemu/aarch64-virt/linux.config"
> +             BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y

> +             BR2_TARGET_ROOTFS_CPIO=y
> +             BR2_TARGET_ROOTFS_CPIO_GZIP=y
> +             # BR2_TARGET_ROOTFS_TAR is not set
> +             BR2_PACKAGE_COMPILER_RT=y
> +             BR2_PACKAGE_LLVM=y

nit: this is not the order after 'make savedefconfig'


[1] http://patchwork.ozlabs.org/patch/912351/
[2] http://patchwork.ozlabs.org/patch/912354/
[3] https://gitlab.com/RicardoMartincoski/buildroot/commit/62a6f7323149b8ea283962c70b14c47cd4b4bb78
[4] https://gitlab.com/RicardoMartincoski/buildroot/blob/61dca1a1246dcda17ecb1e73fc8fa4d9c1921239/support/testing/tests/package/test_clang.py
[5] http://patchwork.ozlabs.org/patch/999190/


Regards,
Ricardo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 3/5] support/testing/infra/emulator.py: support aarch64
  2018-11-16 22:13   ` Ricardo Martincoski
@ 2018-11-17  0:57     ` Matthew Weber
  2018-11-19  2:22       ` [Buildroot] Builtin kernel images for the testing infra Ricardo Martincoski
  0 siblings, 1 reply; 20+ messages in thread
From: Matthew Weber @ 2018-11-17  0:57 UTC (permalink / raw)
  To: buildroot

Ricardo,

On Fri, Nov 16, 2018 at 4:13 PM Ricardo Martincoski
<ricardo.martincoski@gmail.com> wrote:
>
> Hello,
>
> + Thomas
>
> On Tue, Nov 13, 2018 at 09:02 PM, Matt Weber wrote:
>
> >  - Add the condition under the -kernel assignment for aarch64
> >  - Tested with a defconfig simliar to qemu_aarch64_virt_defconfig
>
> s/simliar/similar

Noted.

>
> >
> > Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> > ---
> >  support/testing/infra/emulator.py | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
> > index 802e89d..8e5a7e9 100644
> > --- a/support/testing/infra/emulator.py
> > +++ b/support/testing/infra/emulator.py
> > @@ -62,7 +62,9 @@ class Emulator(object):
> >                      kernel = infra.download(self.downloaddir,
> >                                              "kernel-versatile")
> >                      qemu_cmd += ["-M", "versatilepb"]
> > -
> > +            elif arch == "aarch64":
> > +                kernel_cmdline.append("console=ttyAMA0")
> > +                qemu_cmd += ["-M", "virt", "-cpu", "cortex-a53"]
>
> We don't really need this code here.
> You could do this in the test case:
>
> self.emulator.boot(arch="aarch64",
>                    kernel=kern,
>                    kernel_cmdline=["console=ttyAMA0"],
>                    options=["-M", "virt", "-cpu", "cortex-a53", "-m", "512", "-initrd", img])
>
> If we think more and more test cases would need/want this we could add this
> here, preferably as a "builtin" pre-compiled kernel for aarch64.
>
> But for now, I suggest to move this code to the test case.

I think I'm good either way and asked a bit about this during the last
developers meeting.  I just need to know if the preference is to
provide a kernel or build it each time.  If we start using more
pre-builts, probably need a way to version control those and
streamline submission?

For this test it is quite time consuming already so the kernel build
is in the noise.  Plus your point is valid that we don't have a least
two examples where we would have this in common (yet).  I'll
consolidate this in with the test in my next version.

Thanks for the review!
Matt

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 4/5] support/testing/tests: CLANG compiler-rt runtime test
  2018-11-16 23:45   ` Ricardo Martincoski
@ 2018-11-17  1:15     ` Matthew Weber
  2018-11-17  2:30       ` Matthew Weber
  0 siblings, 1 reply; 20+ messages in thread
From: Matthew Weber @ 2018-11-17  1:15 UTC (permalink / raw)
  To: buildroot

Ricardo,

On Fri, Nov 16, 2018 at 5:45 PM Ricardo Martincoski
<ricardo.martincoski@gmail.com> wrote:
>
> Hello,
>
> In overall it looks good.
>
> On Tue, Nov 13, 2018 at 09:02 PM, Matt Weber wrote:
>
> > This patch adds a test case that
> >  1) Builds the complete LLVM and CLANG set of host tools
> >  2) Cross-compiles the compiler-rt runtime using CLANG
> >  3) Builds a cross-compiled application using CLANG and the libfuzzer
> >     compiler-rt library.
> >  4) Executes the fuzz application on target and checkes expected output
> >
> > Credit to the fuzzer test suite for the tutorial example used as the
> > fuzz test application.
> > https://github.com/google/fuzzer-test-suite
> >
> > Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> > ---
> >  support/testing/tests/package/test_clang.py | 93 +++++++++++++++++++++++++++++
> >  1 file changed, 93 insertions(+)
> >  create mode 100644 support/testing/tests/package/test_clang.py
>
> The change to .gitlab-ci.yml (resulting from 'make .gitlab-ci.yml') is missing.

Ah good catch.

>
> >
> > diff --git a/support/testing/tests/package/test_clang.py b/support/testing/tests/package/test_clang.py
> > new file mode 100644
> > index 0000000..9c42c0d
> > --- /dev/null
> > +++ b/support/testing/tests/package/test_clang.py
> > @@ -0,0 +1,93 @@
> > +import os
> > +import tempfile
> > +import subprocess
> > +import shutil
> > +
> > +import infra.basetest
> > +
> > +FUZZ_TIMEOUT = 120
> > +
> > +
> > +class TestClangBase(infra.basetest.BRTest):
> > +
> > +    def login(self):
> > +        img = os.path.join(self.builddir, "images", "rootfs.cpio.gz")
> > +        kern = os.path.join(self.builddir, "images", "Image")
> > +        # Sanitizers overallocate memory and the minimum that seemed to work was 512MB
> > +        self.emulator.boot(arch="aarch64",
> > +                           kernel=kern,
> > +                           options=["-m", "512", "-initrd", img])
>
> Here you could merge in the contents of patch 3, as I replied there.

I've responded to that one.  I think I'm good either way and plan to
pull the changes over.  I was curious about revision control of the
cases we do pre-builts (didn't realize we had this images pre-built
until I dug into adding this arch config).

>
> > +        self.emulator.login()
> > +
> > +    def build_test_prog(self):
> > +        hostdir = os.path.join(self.builddir, 'host')
> > +        linkerdir = os.path.join(hostdir, 'opt', 'ext-toolchain')
> > +        stagingdir = os.path.join(self.builddir, 'staging')
> > +        env = os.environ.copy()
> > +        env["PATH"] = "{}:".format(os.path.join(hostdir, 'bin')) + env["PATH"]
> > +        clangcpp = os.path.join(hostdir, 'bin', 'clang++')
> > +        workdir = os.path.join(tempfile.mkdtemp(suffix='-br2-testing-compiler-rt'))
> > +        fuzz_src = os.path.join(workdir, 'fuzz_me.cc')
> > +        fuzz_bin = os.path.join(workdir, 'fuzz_me')
> > +        with open(fuzz_src, 'w+') as source_file:
> > +            source_file.write('#include <stdint.h>\n')
> > +            source_file.write('#include <stddef.h>\n')
> > +            source_file.write('bool FuzzMe(const uint8_t *Data, size_t DataSize) {\n')
> > +            source_file.write('  return DataSize >= 3 &&\n')
> > +            source_file.write('      Data[0] == \'F\' &&\n')
> > +            source_file.write('      Data[1] == \'U\' &&\n')
> > +            source_file.write('      Data[2] == \'Z\' &&\n')
> > +            source_file.write('      Data[3] == \'Z\';\n')
> > +            source_file.write('}\n')
> > +            source_file.write('extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {\n')
> > +            source_file.write('  FuzzMe(Data, Size);\n')
> > +            source_file.write('  return 0;\n')
> > +            source_file.write('}\n')
>
> I dislike this a bit for 2 reasons:
>
> 1) The contents of the file would be much more readable in a separate file. See
>    for example the changes introduced by commit "ee6b37cf87 support/testing: use
>    TestPythonPackageBase for python-incremental";
>
> 2) This is a package for the target. The most correct way of handling this IMO
>    is to use a package in a br2-external.
>    Unfortunately this support is not yet merged to the test infra.
>    If you like the idea, you could pull and resend 2 patches of mine: [1] [2]

Will take a look, I've got plans to send out a systemd/docker test
case and a br2_external would help.....

>    Here is a sketch, poorly tested because... well... this test case takes a lot
>    of time to run: [3]
>    And if you follow this road, you could even move the login() method and use a
>    single class: [4]
>
> What do you think about this?

Thanks for that sketch, I'll get something under test for the next version.

>
> [snip]
> > +class TestClangCompilerRT(TestClangBase):
> > +    config = \
> > +             """
>
> I see that you used test_rust as base.
> This test (rust) is the last remaining using a defconfig fragment with
> non-standard style.
> I just sent a patch to fix it: [5]
> Could do the same here?
>

Sure

> > +             BR2_aarch64=y
>
> > +             BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
> > +             BR2_TOOLCHAIN_EXTERNAL=y
>
> nit: this is not the order after 'make savedefconfig'
>

Noted.

> > +             BR2_LINUX_KERNEL=y
> > +             BR2_LINUX_KERNEL_CUSTOM_VERSION=y
> > +             BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.16.7"
> > +             BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y
> > +             BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="board/qemu/aarch64-virt/linux.config"
> > +             BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
>
> > +             BR2_TARGET_ROOTFS_CPIO=y
> > +             BR2_TARGET_ROOTFS_CPIO_GZIP=y
> > +             # BR2_TARGET_ROOTFS_TAR is not set
> > +             BR2_PACKAGE_COMPILER_RT=y
> > +             BR2_PACKAGE_LLVM=y
>
> nit: this is not the order after 'make savedefconfig'
>

Noted.

Appreciate the review,

Matt

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 4/5] support/testing/tests: CLANG compiler-rt runtime test
  2018-11-17  1:15     ` Matthew Weber
@ 2018-11-17  2:30       ` Matthew Weber
  2018-11-19  1:31         ` Ricardo Martincoski
  0 siblings, 1 reply; 20+ messages in thread
From: Matthew Weber @ 2018-11-17  2:30 UTC (permalink / raw)
  To: buildroot

Ricardo,

On Fri, Nov 16, 2018 at 7:15 PM Matthew Weber
<matthew.weber@rockwellcollins.com> wrote:
>
> Ricardo,
>
> On Fri, Nov 16, 2018 at 5:45 PM Ricardo Martincoski
> <ricardo.martincoski@gmail.com> wrote:
> >
> > Hello,
> >
> > In overall it looks good.
> >
> > On Tue, Nov 13, 2018 at 09:02 PM, Matt Weber wrote:
> >
> > > This patch adds a test case that
> > >  1) Builds the complete LLVM and CLANG set of host tools
> > >  2) Cross-compiles the compiler-rt runtime using CLANG
> > >  3) Builds a cross-compiled application using CLANG and the libfuzzer
> > >     compiler-rt library.
> > >  4) Executes the fuzz application on target and checkes expected output
> > >
> > > Credit to the fuzzer test suite for the tutorial example used as the
> > > fuzz test application.
> > > https://github.com/google/fuzzer-test-suite
> > >
> > > Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> > > ---
> > >  support/testing/tests/package/test_clang.py | 93 +++++++++++++++++++++++++++++
> > >  1 file changed, 93 insertions(+)
> > >  create mode 100644 support/testing/tests/package/test_clang.py
> >
> > The change to .gitlab-ci.yml (resulting from 'make .gitlab-ci.yml') is missing.
>
> Ah good catch.
>
> >
> > >
> > > diff --git a/support/testing/tests/package/test_clang.py b/support/testing/tests/package/test_clang.py
> > > new file mode 100644
> > > index 0000000..9c42c0d
> > > --- /dev/null
> > > +++ b/support/testing/tests/package/test_clang.py
> > > @@ -0,0 +1,93 @@
> > > +import os
> > > +import tempfile
> > > +import subprocess
> > > +import shutil
> > > +
> > > +import infra.basetest
> > > +
> > > +FUZZ_TIMEOUT = 120
> > > +
> > > +
> > > +class TestClangBase(infra.basetest.BRTest):
> > > +
> > > +    def login(self):
> > > +        img = os.path.join(self.builddir, "images", "rootfs.cpio.gz")
> > > +        kern = os.path.join(self.builddir, "images", "Image")
> > > +        # Sanitizers overallocate memory and the minimum that seemed to work was 512MB
> > > +        self.emulator.boot(arch="aarch64",
> > > +                           kernel=kern,
> > > +                           options=["-m", "512", "-initrd", img])
> >
> > Here you could merge in the contents of patch 3, as I replied there.
>
> I've responded to that one.  I think I'm good either way and plan to
> pull the changes over.  I was curious about revision control of the
> cases we do pre-builts (didn't realize we had this images pre-built
> until I dug into adding this arch config).
>
> >
> > > +        self.emulator.login()
> > > +
> > > +    def build_test_prog(self):
> > > +        hostdir = os.path.join(self.builddir, 'host')
> > > +        linkerdir = os.path.join(hostdir, 'opt', 'ext-toolchain')
> > > +        stagingdir = os.path.join(self.builddir, 'staging')
> > > +        env = os.environ.copy()
> > > +        env["PATH"] = "{}:".format(os.path.join(hostdir, 'bin')) + env["PATH"]
> > > +        clangcpp = os.path.join(hostdir, 'bin', 'clang++')
> > > +        workdir = os.path.join(tempfile.mkdtemp(suffix='-br2-testing-compiler-rt'))
> > > +        fuzz_src = os.path.join(workdir, 'fuzz_me.cc')
> > > +        fuzz_bin = os.path.join(workdir, 'fuzz_me')
> > > +        with open(fuzz_src, 'w+') as source_file:
> > > +            source_file.write('#include <stdint.h>\n')
> > > +            source_file.write('#include <stddef.h>\n')
> > > +            source_file.write('bool FuzzMe(const uint8_t *Data, size_t DataSize) {\n')
> > > +            source_file.write('  return DataSize >= 3 &&\n')
> > > +            source_file.write('      Data[0] == \'F\' &&\n')
> > > +            source_file.write('      Data[1] == \'U\' &&\n')
> > > +            source_file.write('      Data[2] == \'Z\' &&\n')
> > > +            source_file.write('      Data[3] == \'Z\';\n')
> > > +            source_file.write('}\n')
> > > +            source_file.write('extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {\n')
> > > +            source_file.write('  FuzzMe(Data, Size);\n')
> > > +            source_file.write('  return 0;\n')
> > > +            source_file.write('}\n')
> >
> > I dislike this a bit for 2 reasons:
> >
> > 1) The contents of the file would be much more readable in a separate file. See
> >    for example the changes introduced by commit "ee6b37cf87 support/testing: use
> >    TestPythonPackageBase for python-incremental";
> >
> > 2) This is a package for the target. The most correct way of handling this IMO
> >    is to use a package in a br2-external.
> >    Unfortunately this support is not yet merged to the test infra.
> >    If you like the idea, you could pull and resend 2 patches of mine: [1] [2]
>
> Will take a look, I've got plans to send out a systemd/docker test
> case and a br2_external would help.....
>
> >    Here is a sketch, poorly tested because... well... this test case takes a lot
> >    of time to run: [3]
> >    And if you follow this road, you could even move the login() method and use a
> >    single class: [4]
> >
> > What do you think about this?
>
> Thanks for that sketch, I'll get something under test for the next version.

I've got a v2 tested and read to send out (You sketch was quite
complete and worked).  One general question came to mind.  Why don't
we just create a single br2_external for support/test?

>
> >
> > [snip]
> > > +class TestClangCompilerRT(TestClangBase):
> > > +    config = \
> > > +             """
> >
> > I see that you used test_rust as base.
> > This test (rust) is the last remaining using a defconfig fragment with
> > non-standard style.
> > I just sent a patch to fix it: [5]
> > Could do the same here?
> >
>
> Sure
>
> > > +             BR2_aarch64=y
> >
> > > +             BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
> > > +             BR2_TOOLCHAIN_EXTERNAL=y
> >
> > nit: this is not the order after 'make savedefconfig'
> >
>
> Noted.
>
> > > +             BR2_LINUX_KERNEL=y
> > > +             BR2_LINUX_KERNEL_CUSTOM_VERSION=y
> > > +             BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.16.7"
> > > +             BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y
> > > +             BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="board/qemu/aarch64-virt/linux.config"
> > > +             BR2_LINUX_KERNEL_NEEDS_HOST_OPENSSL=y
> >
> > > +             BR2_TARGET_ROOTFS_CPIO=y
> > > +             BR2_TARGET_ROOTFS_CPIO_GZIP=y
> > > +             # BR2_TARGET_ROOTFS_TAR is not set
> > > +             BR2_PACKAGE_COMPILER_RT=y
> > > +             BR2_PACKAGE_LLVM=y
> >
> > nit: this is not the order after 'make savedefconfig'
> >
>
> Noted.
>
> Appreciate the review,
>
> Matt

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH 4/5] support/testing/tests: CLANG compiler-rt runtime test
  2018-11-17  2:30       ` Matthew Weber
@ 2018-11-19  1:31         ` Ricardo Martincoski
  0 siblings, 0 replies; 20+ messages in thread
From: Ricardo Martincoski @ 2018-11-19  1:31 UTC (permalink / raw)
  To: buildroot

Hello Matt,

+ Arnout
+ Thomas
+ Yann

On Sat, Nov 17, 2018 at 12:30 AM, Matthew Weber wrote:

[snip]
> I've got a v2 tested and read to send out (You sketch was quite
> complete and worked).  One general question came to mind.  Why don't
> we just create a single br2_external for support/test?

While keeping them separate, there is no room for interaction between the
fixtures for unrelated test cases.
Each br2-external will be simple.

If we end up moving each test case to the folder of the package it tests (I
hope we do) the fixtures for the test case can be in a subfolder of the package.
And a few years from now, when we remove a package that have test cases, we just
need to remove the entire package folder, because the package folder contains
everything related to it.

There is a small price to pay: the 3 small files per br2-external.
But not all test cases will use a br2-external, only a subset. All current 70
test cases don't need it.


Regards,
Ricardo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] Builtin kernel images for the testing infra
  2018-11-17  0:57     ` Matthew Weber
@ 2018-11-19  2:22       ` Ricardo Martincoski
  2018-11-19  8:29         ` Thomas Petazzoni
  0 siblings, 1 reply; 20+ messages in thread
From: Ricardo Martincoski @ 2018-11-19  2:22 UTC (permalink / raw)
  To: buildroot

Hello,

I renamed the mail thread (much easier to search for, a few months from now):
-[PATCH 3/5] support/testing/infra/emulator.py: support aarch64
+Builtin kernel images for the testing infra

Warning: a lot of opinions from my side. I certainly missed something.

On Fri, Nov 16, 2018 at 10:57 PM, Matthew Weber wrote:

> On Fri, Nov 16, 2018 at 4:13 PM Ricardo Martincoski
> <ricardo.martincoski@gmail.com> wrote:
[snip]
>> If we think more and more test cases would need/want this we could add this
>> here, preferably as a "builtin" pre-compiled kernel for aarch64.
>>
>> But for now, I suggest to move this code to the test case.
> 
> I think I'm good either way and asked a bit about this during the last
> developers meeting.  I just need to know if the preference is to
> provide a kernel or build it each time.  If we start using more
> pre-builts, probably need a way to version control those and
> streamline submission?

As I see it, these are the steps:
1 - prefer to reuse the pre-built kernel images that already exist whenever
    possible;
2 - if that is not enough, build one, preferably based on an existing
    qemu...defconfig;
3 - when many tests need to build a kernel, check the commonalities between
    them and provide a new pre-built that fits many tests;
4 - back to 1;
And we are currently in step 2, near to go to step 3.
But that is only my opinion, of course.


Concerning new builtin images, if you want to add a new one, I suppose you
could do this:
 - generate the new image and store it in a public temporary url;
 - add the support in the test infra for this new image and use it in at least
   one test case;
 - test your test case by placing the image in the test-dl or your usual
   BR2_DL_DIR folder. It works even on GitLab CI!, see [1];
 - send the patch for review adding a note below "---" with the url for the
   image to be stored in the current artifacts url;


Since you mentioned version control...
Maybe, in the long term, we could create a separate repo, i.e.
buildroot-runtime-tests-binaries in GitHub and accept merges.
I think people in the list would not like to receive patches with 2-5 MB.
A .txt file along each binary stating how the image was generated (i.e. the
sha1 of buildroot and defconfig name, and even the url for a build in GitLab CI)
would be nice IMO.
Just an idea/example.


Two more questions came to mind when thinking about adding more builtin images:

What is the naming to use?
I am happy with the current one: kernel-versatile, and similar.
And when we need to generate a new one for the same arch (actually a machine
that qemu supports) with newer kernel version, the kernel version is a good
suffix IMO.

What kernel config to use?
Should we trim down to have a small image? Should we add stuff to allow maximum
reuse? IMO, using the default kernel intree defconfig selected by our qemu
defconfigs seems a good compromise.
Looking at the images generated at [2]:
versatile = 2.6 MB
vexpress  = 4.1 MB
aarch64   = 6.7 MB
And comparing to the images currently used by the test infra:
versatile = 2.0 MB
vexpress  = 3.3 MB
Those sizes seem acceptable to me.
Notice I did not tested using such images with the test infra.


[1] https://gitlab.com/RicardoMartincoski/buildroot/commit/e226e5a0a1918670fd688cb268176da589ca2fae
[2] https://gitlab.com/buildroot.org/buildroot/pipelines/36081029

Regards,
Ricardo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] Builtin kernel images for the testing infra
  2018-11-19  2:22       ` [Buildroot] Builtin kernel images for the testing infra Ricardo Martincoski
@ 2018-11-19  8:29         ` Thomas Petazzoni
  2018-11-22 23:02           ` Arnout Vandecappelle
  0 siblings, 1 reply; 20+ messages in thread
From: Thomas Petazzoni @ 2018-11-19  8:29 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon, 19 Nov 2018 00:22:24 -0200, Ricardo Martincoski wrote:

> As I see it, these are the steps:
> 1 - prefer to reuse the pre-built kernel images that already exist whenever
>     possible;
> 2 - if that is not enough, build one, preferably based on an existing
>     qemu...defconfig;
> 3 - when many tests need to build a kernel, check the commonalities between
>     them and provide a new pre-built that fits many tests;
> 4 - back to 1;
> And we are currently in step 2, near to go to step 3.
> But that is only my opinion, of course.

Yes, fully agreed. The idea of those pre-built kernel images is to save
time. For a purely user-space test such as the Python test cases, or
Lua test cases, it already takes quite some time to rebuild the Python
interpreter for each test case, it would be annoying to rebuild the
kernel as well.

So it makes sense to provide a few pre-built kernel images for a
variety of architectures, with some "sane" kernel configuration.

Of course, this doesn't prevent from adding other test cases that will
build the Linux kernel:

 - To test the Buildroot Linux kernel packaging
 - Because we want to build some Buildroot packages that provide kernel
   modules, or need some very special kernel configuration options to
   be set

> Concerning new builtin images, if you want to add a new one, I suppose you
> could do this:
>  - generate the new image and store it in a public temporary url;
>  - add the support in the test infra for this new image and use it in at least
>    one test case;
>  - test your test case by placing the image in the test-dl or your usual
>    BR2_DL_DIR folder. It works even on GitLab CI!, see [1];
>  - send the patch for review adding a note below "---" with the url for the
>    image to be stored in the current artifacts url;

Absolutely.

> Since you mentioned version control...
> Maybe, in the long term, we could create a separate repo, i.e.
> buildroot-runtime-tests-binaries in GitHub and accept merges.
> I think people in the list would not like to receive patches with 2-5 MB.
> A .txt file along each binary stating how the image was generated (i.e. the
> sha1 of buildroot and defconfig name, and even the url for a build in GitLab CI)
> would be nice IMO.
> Just an idea/example.

The way we handle those artifacts right now is very basic, and there
are certainly ways of improving that over time.

> Should we trim down to have a small image? Should we add stuff to allow maximum
> reuse? IMO, using the default kernel intree defconfig selected by our qemu
> defconfigs seems a good compromise.

Agree.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] Builtin kernel images for the testing infra
  2018-11-19  8:29         ` Thomas Petazzoni
@ 2018-11-22 23:02           ` Arnout Vandecappelle
  2018-11-23  1:34             ` Matthew Weber
  0 siblings, 1 reply; 20+ messages in thread
From: Arnout Vandecappelle @ 2018-11-22 23:02 UTC (permalink / raw)
  To: buildroot



On 19/11/2018 09:29, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 19 Nov 2018 00:22:24 -0200, Ricardo Martincoski wrote:
> 
>> As I see it, these are the steps:
>> 1 - prefer to reuse the pre-built kernel images that already exist whenever
>>     possible;
>> 2 - if that is not enough, build one, preferably based on an existing
>>     qemu...defconfig;
>> 3 - when many tests need to build a kernel, check the commonalities between
>>     them and provide a new pre-built that fits many tests;
>> 4 - back to 1;
>> And we are currently in step 2, near to go to step 3.
>> But that is only my opinion, of course.
> 
> Yes, fully agreed. The idea of those pre-built kernel images is to save
> time. For a purely user-space test such as the Python test cases, or
> Lua test cases, it already takes quite some time to rebuild the Python
> interpreter for each test case, it would be annoying to rebuild the
> kernel as well.
> 
> So it makes sense to provide a few pre-built kernel images for a
> variety of architectures, with some "sane" kernel configuration.
> 
> Of course, this doesn't prevent from adding other test cases that will
> build the Linux kernel:
> 
>  - To test the Buildroot Linux kernel packaging
>  - Because we want to build some Buildroot packages that provide kernel
>    modules, or need some very special kernel configuration options to
>    be set
> 
>> Concerning new builtin images, if you want to add a new one, I suppose you
>> could do this:
>>  - generate the new image and store it in a public temporary url;
>>  - add the support in the test infra for this new image and use it in at least
>>    one test case;
>>  - test your test case by placing the image in the test-dl or your usual
>>    BR2_DL_DIR folder. It works even on GitLab CI!, see [1];
>>  - send the patch for review adding a note below "---" with the url for the
>>    image to be stored in the current artifacts url;
> 
> Absolutely.
> 
>> Since you mentioned version control...
>> Maybe, in the long term, we could create a separate repo, i.e.
>> buildroot-runtime-tests-binaries in GitHub and accept merges.
>> I think people in the list would not like to receive patches with 2-5 MB.
>> A .txt file along each binary stating how the image was generated (i.e. the
>> sha1 of buildroot and defconfig name, and even the url for a build in GitLab CI)
>> would be nice IMO.
>> Just an idea/example.
> 
> The way we handle those artifacts right now is very basic, and there
> are certainly ways of improving that over time.

 We could use gitlab pipelines to handle this. Define gitlab jobs to generate
the kernel (or whatever), and add dependencies on those.

 The test should then check if the kernel is available, and if not, print
instructions on where to get them.

 Regards,
 Arnout


> 
>> Should we trim down to have a small image? Should we add stuff to allow maximum
>> reuse? IMO, using the default kernel intree defconfig selected by our qemu
>> defconfigs seems a good compromise.
> 
> Agree.
> 
> Best regards,
> 
> Thomas
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] Builtin kernel images for the testing infra
  2018-11-22 23:02           ` Arnout Vandecappelle
@ 2018-11-23  1:34             ` Matthew Weber
  0 siblings, 0 replies; 20+ messages in thread
From: Matthew Weber @ 2018-11-23  1:34 UTC (permalink / raw)
  To: buildroot

Arnout,


On Thu, Nov 22, 2018 at 5:02 PM Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
>
> On 19/11/2018 09:29, Thomas Petazzoni wrote:
> > Hello,
> >
> > On Mon, 19 Nov 2018 00:22:24 -0200, Ricardo Martincoski wrote:
> >
> >> As I see it, these are the steps:
> >> 1 - prefer to reuse the pre-built kernel images that already exist whenever
> >>     possible;
> >> 2 - if that is not enough, build one, preferably based on an existing
> >>     qemu...defconfig;
> >> 3 - when many tests need to build a kernel, check the commonalities between
> >>     them and provide a new pre-built that fits many tests;
> >> 4 - back to 1;
> >> And we are currently in step 2, near to go to step 3.
> >> But that is only my opinion, of course.
> >
> > Yes, fully agreed. The idea of those pre-built kernel images is to save
> > time. For a purely user-space test such as the Python test cases, or
> > Lua test cases, it already takes quite some time to rebuild the Python
> > interpreter for each test case, it would be annoying to rebuild the
> > kernel as well.
> >
> > So it makes sense to provide a few pre-built kernel images for a
> > variety of architectures, with some "sane" kernel configuration.
> >
> > Of course, this doesn't prevent from adding other test cases that will
> > build the Linux kernel:
> >
> >  - To test the Buildroot Linux kernel packaging
> >  - Because we want to build some Buildroot packages that provide kernel
> >    modules, or need some very special kernel configuration options to
> >    be set
> >
> >> Concerning new builtin images, if you want to add a new one, I suppose you
> >> could do this:
> >>  - generate the new image and store it in a public temporary url;
> >>  - add the support in the test infra for this new image and use it in at least
> >>    one test case;
> >>  - test your test case by placing the image in the test-dl or your usual
> >>    BR2_DL_DIR folder. It works even on GitLab CI!, see [1];
> >>  - send the patch for review adding a note below "---" with the url for the
> >>    image to be stored in the current artifacts url;
> >
> > Absolutely.
> >
> >> Since you mentioned version control...
> >> Maybe, in the long term, we could create a separate repo, i.e.
> >> buildroot-runtime-tests-binaries in GitHub and accept merges.
> >> I think people in the list would not like to receive patches with 2-5 MB.
> >> A .txt file along each binary stating how the image was generated (i.e. the
> >> sha1 of buildroot and defconfig name, and even the url for a build in GitLab CI)
> >> would be nice IMO.
> >> Just an idea/example.
> >
> > The way we handle those artifacts right now is very basic, and there
> > are certainly ways of improving that over time.
>
>  We could use gitlab pipelines to handle this. Define gitlab jobs to generate
> the kernel (or whatever), and add dependencies on those.
>
>  The test should then check if the kernel is available, and if not, print
> instructions on where to get them.
>

Maybe this is an opportunity to use the images we're building as part
of the other defconfig test builds?  I'm sure a few kernels would need
some options enabled for virtualization but generally I bet the
images/artifacts would just need to be archived off in a fashion we
could pull from during the run-time tests.  It would give some
coverage of it those images work.

Matt

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-11-23  1:34 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-13 23:02 [Buildroot] [PATCH 1/5] package/compiler-rt: new package Matt Weber
2018-11-13 23:02 ` [Buildroot] [PATCH 2/5] package/llvm: install target binary/debug tools Matt Weber
2018-11-15 17:05   ` Romain Naour
2018-11-13 23:02 ` [Buildroot] [PATCH 3/5] support/testing/infra/emulator.py: support aarch64 Matt Weber
2018-11-16 22:13   ` Ricardo Martincoski
2018-11-17  0:57     ` Matthew Weber
2018-11-19  2:22       ` [Buildroot] Builtin kernel images for the testing infra Ricardo Martincoski
2018-11-19  8:29         ` Thomas Petazzoni
2018-11-22 23:02           ` Arnout Vandecappelle
2018-11-23  1:34             ` Matthew Weber
2018-11-13 23:02 ` [Buildroot] [PATCH 4/5] support/testing/tests: CLANG compiler-rt runtime test Matt Weber
2018-11-16 23:45   ` Ricardo Martincoski
2018-11-17  1:15     ` Matthew Weber
2018-11-17  2:30       ` Matthew Weber
2018-11-19  1:31         ` Ricardo Martincoski
2018-11-13 23:02 ` [Buildroot] [PATCH 5/5] llvm/clang: add note about version bumping together Matt Weber
2018-11-15 16:32   ` Romain Naour
2018-11-14  3:07 ` [Buildroot] [PATCH 1/5] package/compiler-rt: new package Matthew Weber
2018-11-15 16:46 ` Romain Naour
2018-11-15 20:41   ` Matthew Weber

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.