Skip to contentSkip to conversation

Icecream fixes

PATCH New Joshua Watt 1 participant 8 messages Apr 11, 2018

lore
Share & tools
lore
#0 lore raw

cover Icecream fixes

Fixes a number of bugs in Icecream toolchain generation

Joshua Watt (7):
icecc-create-env: Allow logging to a file
icecc-create-env: Fix RUNPATH files
icecc-create-env: Fix library interpreter usage
icecc-create-env: Add extra tools option
icecc.bbclass: Add ICECC_ENV_DEBUG variable
icecc.bbclass: Improve error reporting
icecc.bbclass: Bump version number

 meta/classes/icecc.bbclass                         |  26 ++-
 .../icecc-create-env/icecc-create-env              | 202 ++++++++++++++-------
 2 files changed, 159 insertions(+), 69 deletions(-)
Signature · 1 line
2.14.3
#1 lore raw

PATCH 1/7 icecc-create-env: Allow logging to a file

Modifies the icecc-create-env script so that it can log output to a log file. In addition, a --debug flag is added that allows verbose logging. Finally, the silent flag was removed since it was never used in icecc.bbclass

---
 .../icecc-create-env/icecc-create-env              | 74 ++++++++++++++--------
 1 file changed, 49 insertions(+), 25 deletions(-)
meta/recipes-devtools/icecc-create-env/icecc-create-env/icecc-create-env +49 -25
index 537e38a9ba0..074c7675c0b 100755
@@ -7,6 +7,24 @@
 target_paths=
 target_aliases=
 
+# Always prints, optionally to a log file
+print_output ()
+{
+    if test -n "$log_path"; then
+        echo "$@" | tee -a "$log_path"
+    else
+        echo "$@"
+    fi
+}
+
+# Only prints if the debug flag is specified
+print_debug ()
+{
+    if test -n "$debug"; then
+        print_output "$@"
+    fi
+}
+
 is_dynamic_elf ()
 {
     # Is the file an dynamically linked ELF executable?
@@ -54,6 +72,7 @@ add_alias ()
             *" $alias "*)
                 ;;
             *)
+                print_debug "Adding alias '$2' -> '$1'"
                 target_aliases="$target_aliases $alias"
                 ;;
         esac
@@ -123,17 +142,23 @@ add_file ()
     fi
 }
 
-# backward compat
-if test "$1" = "--respect-path"; then
-    shift
-fi
-
-#add a --silent switch to avoid "broken pipe" errors when calling this scipt from within OE
-if test "$1" = "--silent"; then
-    silent=1
+while test -n "$1"; do
+    case "$1" in
+        --respect-path)
+            # Ignore for backward compatability
+            ;;
+        --debug)
+            debug=1
+            ;;
+        --log)
+            do_log=1
+            ;;
+        *)
+            break
+            ;;
+    esac
     shift
-fi
-
+done
 
 added_gcc=$1
 shift
@@ -143,6 +168,11 @@ added_as=$1
 shift
 archive_name=$1
 
+if test -n "$do_log"; then
+    log_path="$archive_name.log"
+    rm -f "$log_path"
+fi
+
 if test -z "$PATCHELF"; then
     PATCHELF=`which patchelf 2> /dev/null`
 fi
@@ -150,22 +180,22 @@ if test -z "$PATCHELF"; then
     PATCHELF=`which patchelf-uninative 2> /dev/null`
 fi
 if test -z "$PATCHELF"; then
-    echo "patchelf is required"
+    print_output "patchelf is required"
     exit 1
 fi
 
 if test -z "$added_gcc" || test -z "$added_gxx" ; then
-    echo "usage: $0 <gcc_path> <g++_path>"
+    print_output "usage: $0 <gcc_path> <g++_path>"
     exit 1
 fi
 
 if ! test -x "$added_gcc" ; then
-    echo "'$added_gcc' is no executable."
+    print_output "'$added_gcc' is not executable."
     exit 1
 fi
 
 if ! test -x "$added_gxx" ; then
-    echo "'$added_gcc' is no executable."
+    print_output "'$added_gcc' is not executable."
     exit 1
 fi
 
@@ -178,7 +208,7 @@ if test -z "$added_as" ; then
     add_file /usr/bin/as /usr/bin/as
 else
     if ! test -x "$added_as" ; then
-        echo "'$added_as' is no executable."
+        print_output "'$added_as' is not executable."
         exit 1
     fi
 
@@ -206,7 +236,7 @@ if test -x /bin/true; then
 elif test -x /usr/bin/true; then
     add_file /usr/bin/true /bin/true
 else
-    echo "'true' not found"
+    print_output "'true' not found"
     exit 1
 fi
 
@@ -266,9 +296,7 @@ if test -z "$archive_name"; then
 
     #calculate md5 and use it as the archive name
     archive_name=`for i in $target_files; do test -f $tempdir/$i && $md5sum $tempdir/$i; done | sed -e 's/ .*$//' | $md5sum | sed -e 's/ .*$//'`.tar.gz || {
-        if test -z "$silent"; then
-            echo "Couldn't compute MD5 sum."
-        fi
+        print_output "Couldn't compute MD5 sum."
         exit 2
     }
     mydir=`pwd`
@@ -283,9 +311,7 @@ else
     fi
 fi
 
-if test -z "$silent"; then
-    echo "creating $archive_name"
-fi
+print_output "creating $archive_name"
 
 cd $tempdir
 # Add everything in the temp directory. Tar doesn't like to be given files with
@@ -293,9 +319,7 @@ cd $tempdir
 # the path prefix past the offending "..". This makes the archive generate
 # incorrectly
 tar -czf "$mydir/$archive_name" . || {
-    if test -z "$silent"; then
-        echo "Couldn't create archive"
-    fi
+    print_output "Couldn't create archive"
     exit 3
 }
 cd ..
Signature · 1 line
2.14.3
#2 lore raw

PATCH 2/7 icecc-create-env: Fix RUNPATH files

In reply to Joshua Watt

Some newer libraries and programs use RUNPATH to specify the library search path. These executables were being skipped by the rpath fixup code because it was grepping the ELF header for RPATH only. A more correct solution is to ask patchelf to report the rpath, as that tool will properly report either RPATH or RUNPATH as appropriate.

---
 .../icecc-create-env/icecc-create-env/icecc-create-env                | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
meta/recipes-devtools/icecc-create-env/icecc-create-env/icecc-create-env +3 -1
index 074c7675c0b..3015f4e2155 100755
@@ -42,11 +42,13 @@ fix_rpath ()
     if ! is_dynamic_elf "$path"; then
         return
     fi
-    local new_rpath="`readelf -w -d "$path" | grep RPATH | \
+    local old_rpath="`$PATCHELF --print-rpath "$path"`"
+    local new_rpath="`echo "$old_rpath" | \
         sed 's/.*\[\(.*\)\]/\1/g' | \
         sed "s,\\\$ORIGIN,/$origin,g"`"
 
     if test -n "$new_rpath"; then
+        print_debug "Converting RPATH '$old_rpath' -> '$new_rpath'"
         $PATCHELF --set-rpath "$new_rpath" "$path"
     fi
 }
Signature · 1 line
2.14.3
#3 lore raw

PATCH 3/7 icecc-create-env: Fix library interpreter usage

In reply to Joshua Watt

Shared libraries sometimes (frequently?) don't have a program interpreter specified. The previous code would fail to find the library dependencies in these cases because no interpreter could be found. Commonly, this meant that if a library depends on another library, it might not be included toolchain because dependency scanning stops with the first one.

Instead, capture the program interpreter from the program or library that starts the dependency chain and use that interpreter to get all of the dependencies in the chain, recursively.

Additionally, if no interpreter can be found, fallback to using ldd

---
 .../icecc-create-env/icecc-create-env              | 112 ++++++++++++++-------
 1 file changed, 77 insertions(+), 35 deletions(-)
meta/recipes-devtools/icecc-create-env/icecc-create-env/icecc-create-env +77 -35
index 3015f4e2155..b88c53a424b 100755
@@ -99,48 +99,90 @@ normalize_path ()
     echo $path
 }
 
-add_file ()
+add_file_common()
+{
+    local p="$1"
+    local path="$2"
+    local alias="$3"
+
+    add_alias "$path" "$p"
+    if test -n "$alias"; then
+        add_alias "$path" "$alias"
+    fi
+
+    add_path "$path" || return 1
+    print_debug "Adding file '$path'"
+
+    return 0
+}
+
+add_deps()
+{
+    local path="$1"
+    local interp="$2"
+
+    if test -n "$interp" && test -x "$interp"; then
+        # Use the dynamic loaders --list argument to list the
+        # dependencies. The program may have a different program
+        # interpreter (typical when using uninative tarballs), which is
+        # why we can't just call ldd.
+        deps="`$interp --list "$path"`"
+    else
+        deps="`ldd "$path"`"
+    fi
+
+    print_debug "Dependencies are:"
+    print_debug "$deps"
+    if test -n "$deps"; then
+        for lib in $deps; do
+            # ldd now outputs ld as /lib/ld-linux.so.xx on current nptl
+            # based glibc this regexp parse the outputs like:
+            # ldd /usr/bin/gcc
+            #         linux-gate.so.1 =>  (0xffffe000)
+            #         libc.so.6 => /lib/tls/libc.so.6 (0xb7e81000)
+            #         /lib/ld-linux.so.2 (0xb7fe8000)
+            # covering both situations ( with => and without )
+            lib="`echo "$lib" | sed -n 's,^[^/]*\(/[^ ]*\).*,\1,p'`"
+
+            test -f "$lib" || continue
+            # Check whether the same library also exists in the parent
+            # directory, and prefer that on the assumption that it is a
+            # more generic one.
+            local baselib=`echo "$lib" | sed 's,\(/[^/]*\)/.*\(/[^/]*\)$,\1\2,'`
+            test -f "$baselib" && lib=$baselib
+            add_dependency "$lib" "$interp"
+        done
+    fi
+}
+
+add_dependency()
 {
     local p=`normalize_path $1`
     # readlink is required for Yocto, so we can use it
     local path=`readlink -f "$p"`
+    local interp="$2"
 
-    add_alias "$path" "$p"
-    if test -n "$2"; then
-        add_alias "$path" "$2"
+    add_file_common "$p" "$path" || return
+
+    if test -x "$path" && is_dynamic_elf "$path"; then
+        add_deps "$path" "$interp"
     fi
+}
 
-    add_path "$path" || return
-
-    if test -x "$path"; then
-        # Only call ldd when it makes sense
-        if is_dynamic_elf "$path"; then
-            # Request the program interpeter (dynamic loader)
-            interp=`readelf -w -l "$path" | grep "Requesting program interpreter:" | sed "s/\s*\[Requesting program interpreter:\s*\(.*\)\]/\1/g"`
-
-            if test -n "$interp" && test -x "$interp"; then
-                # Use the dynamic loaders --list argument to list the
-                # depenencies. The program may have a a different program
-                # interpeter (typical when using uninative tarballs), which is
-                # why we can't just call ldd.
-                #
-                # ldd now outputs ld as /lib/ld-linux.so.xx on current nptl based glibc
-                # this regexp parse the outputs like:
-                # ldd /usr/bin/gcc
-                #         linux-gate.so.1 =>  (0xffffe000)
-                #         libc.so.6 => /lib/tls/libc.so.6 (0xb7e81000)
-                #         /lib/ld-linux.so.2 (0xb7fe8000)
-                # covering both situations ( with => and without )
-                for lib in `$interp --list "$path" | sed -n 's,^[^/]*\(/[^ ]*\).*,\1,p'`; do
-                    test -f "$lib" || continue
-                    # Check wether the same library also exists in the parent directory,
-                    # and prefer that on the assumption that it is a more generic one.
-                    local baselib=`echo "$lib" | sed 's,\(/[^/]*\)/.*\(/[^/]*\)$,\1\2,'`
-                    test -f "$baselib" && lib=$baselib
-                    add_file "$lib"
-                done
-            fi
-        fi
+add_file ()
+{
+    local p=`normalize_path $1`
+    # readlink is required for Yocto, so we can use it
+    local path=`readlink -f "$p"`
+
+    add_file_common "$p" "$path" "$2" || return
+
+    if test -x "$path" && is_dynamic_elf "$path"; then
+        # Request the program interpeter (dynamic loader)
+        interp=`readelf -W -l "$path" | grep "Requesting program interpreter:" | sed "s/\s*\[Requesting program interpreter:\s*\(.*\)\]/\1/g"`
+        print_debug "Interpreter is '$interp'"
+
+        add_deps "$path" "$interp"
     fi
 }
 
Signature · 1 line
2.14.3
#4 lore raw

PATCH 4/7 icecc-create-env: Add extra tools option

In reply to Joshua Watt

It can often be useful to include additional debugging tools the toolchain such as strace. Add an option to include an arbitrary path.

---
 .../icecc-create-env/icecc-create-env/icecc-create-env       | 12 ++++++++++++
 1 file changed, 12 insertions(+)
meta/recipes-devtools/icecc-create-env/icecc-create-env/icecc-create-env +12 -0
index b88c53a424b..64b5e207853 100755
@@ -197,6 +197,9 @@ while test -n "$1"; do
         --log)
             do_log=1
             ;;
+        --extra=*)
+            extra_tools="$extra_tools ${1#--extra=}"
+            ;;
         *)
             break
             ;;
@@ -284,6 +287,15 @@ else
     exit 1
 fi
 
+for extra in $extra_tools; do
+    if test -x "$extra"; then
+        add_file "$extra"
+    else
+        print_output "'$extra' not found"
+        exit 1
+    fi
+done
+
 link_rel ()
 {
     local target="$1"
Signature · 1 line
2.14.3
#5 lore raw

PATCH 5/7 icecc.bbclass: Add ICECC_ENV_DEBUG variable

In reply to Joshua Watt

The ICECC_ENV_DEBUG variable can be set in local.conf to pass additional debugging options to the Icecream toolchain creation script.

---
 meta/classes/icecc.bbclass | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
meta/classes/icecc.bbclass +5 -1
index cab64f52703..aea1095f5fb 100644
@@ -33,6 +33,7 @@ BB_HASHBASE_WHITELIST += "ICECC_PARALLEL_MAKE ICECC_DISABLED ICECC_USER_PACKAGE_
     ICECC_CARET_WORKAROUND ICECC_CFLAGS ICECC_ENV_VERSION \
     ICECC_DEBUG ICECC_LOGFILE ICECC_REPEAT_RATE ICECC_PREFERRED_HOST \
     ICECC_CLANG_REMOTE_CPP ICECC_IGNORE_UNVERIFIED ICECC_TEST_SOCKET \
+    ICECC_ENV_DEBUG \
     "
 
 ICECC_ENV_EXEC ?= "${STAGING_BINDIR_NATIVE}/icecc-create-env"
@@ -57,6 +58,9 @@ ICECC_CFLAGS = ""
 CFLAGS += "${ICECC_CFLAGS}"
 CXXFLAGS += "${ICECC_CFLAGS}"
 
+# Debug flags when generating environments
+ICECC_ENV_DEBUG ??= ""
+
 def icecc_dep_prepend(d):
     # INHIBIT_DEFAULT_DEPS doesn't apply to the patch command.  Whether or  not
     # we need that built is the responsibility of the patch function / class, not
@@ -363,7 +367,7 @@ set_icecc_env() {
         # the ICECC_VERSION generation step must be locked by a mutex
         # in order to prevent race conditions
         if flock -n "${ICECC_VERSION}.lock" \
-            ${ICECC_ENV_EXEC} "${ICECC_CC}" "${ICECC_CXX}" "${ICECC_AS}" "${ICECC_VERSION}"
+            ${ICECC_ENV_EXEC} ${ICECC_ENV_DEBUG} "${ICECC_CC}" "${ICECC_CXX}" "${ICECC_AS}" "${ICECC_VERSION}"
         then
             touch "${ICECC_VERSION}.done"
         elif [ ! wait_for_file "${ICECC_VERSION}.done" 30 ]
Signature · 1 line
2.14.3
#6 lore raw

PATCH 6/7 icecc.bbclass: Improve error reporting

In reply to Joshua Watt

Improve reporting when the icecream environment cannot be created by assigning the flock call a specific error number when the lock fails so it can be distinguished from environment creation errors.

---
 meta/classes/icecc.bbclass | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)
meta/classes/icecc.bbclass +13 -7
index aea1095f5fb..1b58d1f5e52 100644
@@ -366,14 +366,20 @@ set_icecc_env() {
 
         # the ICECC_VERSION generation step must be locked by a mutex
         # in order to prevent race conditions
-        if flock -n "${ICECC_VERSION}.lock" \
-            ${ICECC_ENV_EXEC} ${ICECC_ENV_DEBUG} "${ICECC_CC}" "${ICECC_CXX}" "${ICECC_AS}" "${ICECC_VERSION}"
-        then
+        exit_code=0
+        flock -n -E 10 "${ICECC_VERSION}.lock" \
+            ${ICECC_ENV_EXEC} ${ICECC_ENV_DEBUG} "${ICECC_CC}" "${ICECC_CXX}" "${ICECC_AS}" "${ICECC_VERSION}" || exit_code=$?
+        if [ "$exit_code" -eq 0 ]; then
+            touch "${ICECC_VERSION}.done"
+        elif [ "$exit_code" -eq "10" ]; then
+            if [ ! wait_for_file "${ICECC_VERSION}.done" 30 ]; then
+                # locking failed so wait for ${ICECC_VERSION}.done to appear
+                bbwarn "Timeout waiting for ${ICECC_VERSION}.done"
+                return
+            fi
+        else
+            bbwarn "Could not create icecc environment: $exit_code"
             touch "${ICECC_VERSION}.done"
-        elif [ ! wait_for_file "${ICECC_VERSION}.done" 30 ]
-        then
-            # locking failed so wait for ${ICECC_VERSION}.done to appear
-            bbwarn "Timeout waiting for ${ICECC_VERSION}.done"
             return
         fi
     fi
Signature · 1 line
2.14.3
#7 lore raw

PATCH 7/7 icecc.bbclass: Bump version number

In reply to Joshua Watt

Bump the version number for force remotes to use a newly generated environment, since the old one potentially had a few bugs

---
 meta/classes/icecc.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
meta/classes/icecc.bbclass +1 -1
index 1b58d1f5e52..73eba7a6a96 100644
@@ -45,7 +45,7 @@ ICECC_ENV_EXEC ?= "${STAGING_BINDIR_NATIVE}/icecc-create-env"
 # A useful thing to do for testing Icecream changes locally is to add a
 # subversion in local.conf:
 #  ICECC_ENV_VERSION_append = "-my-ver-1"
-ICECC_ENV_VERSION = "1"
+ICECC_ENV_VERSION = "2"
 
 # Default to disabling the caret workaround, If set to "1" in local.conf, icecc
 # will locally recompile any files that have warnings, which can adversely
Signature · 1 line
2.14.3