OK here's the patch I was talking about... I actually wrote it in Make first and then completely rewrote it, and it's much simpler now.
My rant is that Makefiles should be for dataflow and shell scripts are for actions, and they work together beautifully (makefiles call shell scripts, and sometimes shell scripts call 'make'). But I think .PHONY targets are usually a sign of abusing Makefiles as shell scripts. There's nothing really different about 'make test' vs './test.sh all'. I think 'make test' should be kept as an alias for downstream familiarity, but 'make test_sed' etc. should be removed in favor of './test.sh single sed'. They are preserved in this patch though. I would patch this and try ./test.sh single -asan expr. It's super nice and gives a stack trace of a memory leak. It definitely found leaks before your last patch with 'refree'. I didn't try yet but I think it will find leaks in the string -> int conversion (or it will if we add the proper test cases.) -msan is finding unitialized reads, like in xabspath(), *buf is used without initializing char buf[4096]. -ubsan is finding some integer overflows, and some more stuff I didn't look into deeply yet. Note that this is runtime instrumentation, not static analysis, so they are actually happening. Also I think these renaming would be nice: scripts/test.sh -> scripts/run_tests.sh # helper script for "front end" ./test.sh scripts/runtest.sh -> scripts/test_lib.sh # the thing all the tests source scripts/install.c -> scripts/instlist.c ? Because the binary is generated/instlist? (I didn't do that because I'm not sure how it would patch...) Also there is a TODO in test.sh... there seems to be a couple places where we are grepping toys/*/*.c for info about commands, and I think one is wrong because I get '-toysh' in addition to 'toysh'. This can probably be cleaned up... if you want me to do that in this patch set let me know. Andy On Sun, Mar 20, 2016 at 9:57 PM, Andy Chu <[email protected]> wrote: > On Sun, Mar 20, 2016 at 9:08 PM, Rob Landley <[email protected]> wrote: >> On 03/18/2016 08:24 PM, Andy Chu wrote: >>> This is a pretty big cleanup, still in progress, but I think these >>> parts are ready to merge. >> >> It's a large lump, some bits of which I disagree with. Should I apply >> and then apply an undo patch, or chip off bits? > > Yeah sorry about the big lump, but the original history is super messy > -- there was a lot of churn of going back and forth as I was figuring > things out and learning the code. > > If you read the commit description, it seems disjointed, but > everything is related to producing a "green build" (passing tests). > If you compare "make test" before and after, all the changes show up > there, and I think it's good progress. > > But I have another large lump coming up (call it patch 0004)! It adds > some significant functionality (ASAN and all that, which found some > good bugs), as well as fixing test environment issues. > > There was a recurring theme in the test failures -- under "make test", > you would have a directory in your PATH with ALL toybox binaries. > Under "make test_sed", you would only have sed in that dir. Patches > 1-3 don't fix that but patch 4 does. > > Patch 0004 also addresses some of the issues below. I changed it to > "make generated/single/sed" instead of "make sed", and this eliminates > the namespace clash. I kept "make sed" as a phony target for backward > compatibility, but personally I would prefer to remove them. > > I'll respond in detail to the issues below after I send it out ... I > can definitely change things based on your feedback, but it might have > to be on top of this patch... patch #5 on top of #4 to address your > feedback. > > Andy
From 777b5bdd9395858979a4307e8c3ca48452349b4c Mon Sep 17 00:00:00 2001 From: Andy Chu <[email protected]> Date: Sun, 20 Mar 2016 22:25:55 -0700 Subject: [PATCH 4/4] Build and test changes to support Clang sanitizers. ./test.sh is a new "front end" for running tests in different modes. I originally wrote this patch mostly in make, so you would do 'make asantest_sed', analogous to 'make test_sed'. But .singlemake became much bigger, and really it's silly to auto-generate multiple targets for every command when you can just have a shell script that takes the command as an argument. So now the interface is: ./test.sh all 'make test' is an alias ./test.sh single sed 'make test_sed' is an alias Run ./test.sh without args for details and examples. See the comment at the top of the Makefile for the tree layout. Details: Makefile: Add toybox_asan, toybox_msan, toybox_ubsan targets. make.sh: - Allow outputting only the unstripped binary, as toybox_asan_unstripped would never be useful - Create different obj/ dirs based on CC and CFLAGS. This reduces the need to 'make clean' a lot. (Each of the 3 sanitizers does different, incompatible runtime instrumentation.) single.sh: Generate single tools in generated/single/ rather than the root directory. This prevents us from having to special case 'test' in a lot of places, and also 'make clean' no longer needs code generation. ./test.sh: Fix the host/toybox divergence between the environment in 'make test' and 'make test_$COMMAND'. The latter only had one toybox binary in the PATH, so everything would fall back to host binaries. scripts/test.sh: This is a helper tool now, not to be invoked by the developer. ./test.sh is the front end -- it automatically builds dependent binaries make symlinks, and then invokes scripts/test.sh. Prior to this change you had to run 'make' before 'make test', now you can just to 'make test' from a clean tree if you like. --- Makefile | 62 ++++++++++++++++++- scripts/genconfig.sh | 21 +++---- scripts/make.sh | 35 +++++++---- scripts/single.sh | 24 ++++---- scripts/test.sh | 40 ++++--------- test.sh | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 282 insertions(+), 64 deletions(-) create mode 100755 test.sh diff --git a/Makefile b/Makefile index 2f92a12..60bb2f4 100644 --- a/Makefile +++ b/Makefile @@ -5,9 +5,35 @@ # Note that CC defaults to "cc" so the one in configure doesn't get # used when scripts/make.sh and care called through "make". +# Build tree layout: +# +# git/toybox/ +# toybox # stripped binary +# toybox_unstripped +# toybox_asan # Binaries with runtime instrumentation +# toybox_msan +# toybox_ubsan +# generated/ +# obj-$MD5SUM # object files for each combination of CC and CFLAGS +# # (created by scripts/make.sh) +# single/ # Binaries configured to contain a single command +# cat # (created by scripts/single.sh) +# ls +# ... +# tree/ +# all/ # all commands linked to ../../toybox +# all-asan/ # all commands linked to ../../toybox_asan +# all-msan/ +# all-ubsan/ +# cat/ # like tree/all/, except cat is a real binary +# ls/ +# ... +# +# See ./test.sh for instructions on running these binaries. + HOSTCC?=cc -export CROSS_COMPILE CFLAGS OPTIMIZE LDOPTIMIZE CC HOSTCC V +export CROSS_COMPILE CFLAGS OPTIMIZE LDOPTIMIZE CC HOSTCC V NOSTRIP all: toybox @@ -18,10 +44,39 @@ toybox_stuff: $(KCONFIG_CONFIG) *.[ch] lib/*.[ch] toys/*.h toys/*/*.c scripts/*. toybox toybox_unstripped: toybox_stuff scripts/make.sh +# CLANG_DIR should be set to build and run tests under sanitizers. +SAN_CC = +ifdef CLANG_DIR + SAN_CC = $(CLANG_DIR)/bin/clang +else + SAN_CC = clang +endif + +# Binaries built with Clang sanitizers. All of these should be unstripped +# because they show stack traces at runtime. +toybox_asan: CC = $(SAN_CC) +toybox_asan: CFLAGS = -fsanitize=address -g +toybox_asan: NOSTRIP = 1 +toybox_asan: toybox_stuff + scripts/make.sh toybox_asan + +toybox_msan: CC = $(SAN_CC) +toybox_msan: CFLAGS = -fsanitize=memory -g +toybox_msan: NOSTRIP = 1 +toybox_msan: toybox_stuff + scripts/make.sh toybox_msan + +toybox_ubsan: CC = $(SAN_CC) +toybox_ubsan: CFLAGS = -fsanitize=undefined -fno-omit-frame-pointer -g +toybox_ubsan: NOSTRIP = 1 +toybox_ubsan: toybox_stuff + scripts/make.sh toybox_ubsan + .PHONY: clean distclean baseline bloatcheck install install_flat \ uinstall uninstall_flat test tests help toybox_stuff change \ list list_working list_pending + include kconfig/Makefile -include .singlemake @@ -53,7 +108,8 @@ change: scripts/change.sh clean:: - rm -rf toybox toybox_unstripped generated change .singleconfig* + rm -rf toybox toybox_unstripped toybox_asan toybox_msan toybox_ubsan \ + generated change .singleconfig* distclean: clean rm -f toybox_old .config* .singlemake @@ -61,7 +117,7 @@ distclean: clean test: tests tests: - scripts/test.sh all + ./test.sh all help:: @echo ' toybox - Build toybox.' diff --git a/scripts/genconfig.sh b/scripts/genconfig.sh index 6a15d09..b7883ae 100755 --- a/scripts/genconfig.sh +++ b/scripts/genconfig.sh @@ -151,7 +151,7 @@ print_singlemake() { local working= local pending= - local test_targets= + local phony_targets= while IFS=":" read cmd_src cmd do [ "$cmd" == help ] && continue @@ -163,30 +163,31 @@ print_singlemake() # can be built with 'make test_bin'. [ "$cmd" == test ] && build_name=test_bin - # Print a build target and test target for each command. + # Print a build target and test target for each command. Make a phony + # alias for each build target. cat <<EOF -$build_name: $cmd_src *.[ch] lib/*.[ch] - scripts/single.sh $cmd +generated/single/$cmd: $cmd_src *.[ch] lib/*.[ch] + scripts/single.sh generated/single/$cmd + +$build_name: generated/single/$cmd + @echo "Built generated/single/$cmd" $test_name: - scripts/test.sh single $cmd + ./test.sh single $cmd EOF [ "${cmd_src/pending//}" != "$cmd_src" ] && pending="$pending $cmd" || working="$working $cmd" - test_targets="$test_targets $test_name" + phony_targets="$phony_targets $build_name $test_name" done # Print more targets. cat <<EOF # test_bin builds the 'test' file, not a file named test_bin. And all the rest # of the test targest are phony too. -.PHONY: test_bin $test_targets - -clean:: - rm -f $working $pending +.PHONY: $phony_targets list: @echo $(echo $working $pending | sort_words) diff --git a/scripts/make.sh b/scripts/make.sh index cc85b30..1656fa9 100755 --- a/scripts/make.sh +++ b/scripts/make.sh @@ -10,7 +10,8 @@ export LC_ALL=C set -o pipefail source ./configure -# output ${OUTNAME}_unstripped and and $OUTNAME +# Output ${OUTNAME}_unstripped and $OUTNAME, or just $OUTNAME if NOSTRIP is +# set. OUTNAME=${1:-toybox} [ -z "$KCONFIG_CONFIG" ] && KCONFIG_CONFIG=".config" @@ -31,7 +32,7 @@ do_loudly() # Is anything under directory $2 newer than file $1 isnewer() { - [ ! -z "$(find "$2" -newer "$1" 2>/dev/null || echo yes)" ] + [ ! -z "$(find "$2" -newer "$1" 2>/dev/null || echo yes)" ] } echo "Generate headers from toys/*/*.c..." @@ -117,7 +118,9 @@ fi # LINK needs optlibs.dat, above -LINK="$(echo $LDOPTIMIZE $LDFLAGS -o ${OUTNAME}_unstripped -Wl,--as-needed $(cat generated/optlibs.dat))" +[ -z "$NOSTRIP" ] && LINKOUT=${OUTNAME}_unstripped || LINKOUT=$OUTNAME + +LINK="$(echo $LDOPTIMIZE $LDFLAGS -o ${LINKOUT} -Wl,--as-needed $(cat generated/optlibs.dat))" genbuildsh > generated/build.sh && chmod +x generated/build.sh || exit 1 echo "Make generated/config.h from $KCONFIG_CONFIG." @@ -250,12 +253,19 @@ DOTPROG=. # This is a parallel version of: do_loudly $BUILD $FILES $LINK || exit 1 -X="$(ls -1t generated/obj/* 2>/dev/null | tail -n 1)" +# Create a unique dir for object files based on the compiler and flags. For +# example, toybox and generated/single/sed have the same CFLAGS and can use the +# same objects, but toybox and toybox_asan don't. + +DIR_ID=$(echo "$CC $CFLAGS" | md5sum | cut -d' ' -f1) +OBJDIR="generated/obj-$DIR_ID" + +X="$(ls -1t $OBJDIR/* 2>/dev/null | tail -n 1)" # newest file if [ ! -e "$X" ] || [ ! -z "$(find toys -name "*.h" -newer "$X")" ] then - rm -rf generated/obj && mkdir -p generated/obj || exit 1 + rm -rf $OBJDIR && mkdir -p $OBJDIR || exit 1 else - rm -f generated/obj/{main,lib_help}.o || exit 1 + rm -f $OBJDIR/{main,lib_help}.o || exit 1 fi PENDING= LFILES= @@ -263,11 +273,11 @@ DONE=0 COUNT=0 for i in $FILES do - # build each generated/obj/*.o file in parallel + # build each generated/obj-ID/*.o file in parallel X=${i/lib\//lib_} X=${X##*/} - OUT="generated/obj/${X%%.c}.o" + OUT="$OBJDIR/${X%%.c}.o" LFILES="$LFILES $OUT" [ "$OUT" -nt "$i" ] && continue do_loudly $BUILD -c $i -o $OUT & @@ -299,10 +309,13 @@ done [ $DONE -ne 0 ] && exit 1 do_loudly $BUILD $LFILES $LINK || exit 1 -if [ ! -z "$NOSTRIP" ] || ! do_loudly ${CROSS_COMPILE}strip ${OUTNAME}_unstripped -o $OUTNAME +if [ -z "$NOSTRIP" ] then - echo "strip failed, using unstripped" && cp ${OUTNAME}_unstripped $OUTNAME || - exit 1 + if ! do_loudly ${CROSS_COMPILE}strip ${OUTNAME}_unstripped -o $OUTNAME + then + echo "strip failed, using unstripped" + cp ${OUTNAME}_unstripped $OUTNAME || exit 1 + fi fi # gcc 4.4's strip command is buggy, and doesn't set the executable bit on diff --git a/scripts/single.sh b/scripts/single.sh index a29ff46..a46afea 100755 --- a/scripts/single.sh +++ b/scripts/single.sh @@ -3,13 +3,10 @@ # Build standalone toybox commands. # # Usage: -# scripts/single.sh COMMAND... -# -# The output is put in the repo root, or $PREFIX. +# scripts/single.sh COMMAND_PATH... # # Example: -# # Put grep and sed binaries in this dir -# $ PREFIX=generated/test/bin scripts/single.sh grep sed +# $ scripts/single.sh generated/single/grep generated/single/sed if [ $# -eq 0 ] then @@ -31,27 +28,28 @@ fi # 3) make toybox, and then move it to the command name. export KCONFIG_CONFIG=.singleconfig -for i in "$@" +for out_path in "$@" do - echo -n "$i:" - TOYFILE="$(egrep -l "TOY[(]($i)[ ,]" toys/*/*.c)" + cmd=$(basename $out_path) + echo -n "$cmd:" + TOYFILE="$(egrep -l "TOY[(]($cmd)[ ,]" toys/*/*.c)" if [ -z "$TOYFILE" ] then - echo "Unknown command '$i'" >&2 + echo "Unknown command '$cmd'" >&2 exit 1 fi # Enable stuff this command depends on - DEPENDS="$(sed -n "/^config *$i"'$/,/^$/{s/^[ \t]*depends on //;T;s/[!][A-Z0-9_]*//g;s/ *&& */|/g;p}' $TOYFILE | xargs | tr ' ' '|')" + DEPENDS="$(sed -n "/^config *$cmd"'$/,/^$/{s/^[ \t]*depends on //;T;s/[!][A-Z0-9_]*//g;s/ *&& */|/g;p}' $TOYFILE | xargs | tr ' ' '|')" - NAME=$(echo $i | tr a-z- A-Z_) + NAME=$(echo $cmd | tr a-z- A-Z_) make allnoconfig > /dev/null && sed -ri -e '/CONFIG_TOYBOX/d' \ -e "s/# (CONFIG_($NAME|${NAME}_.*${DEPENDS:+|$DEPENDS})) is not set/\1=y/" \ "$KCONFIG_CONFIG" && echo "# CONFIG_TOYBOX is not set" >> "$KCONFIG_CONFIG" && grep "CONFIG_TOYBOX_" .config >> "$KCONFIG_CONFIG" && - - scripts/make.sh $PREFIX$i || exit 1 + mkdir -p $(dirname $out_path) && + scripts/make.sh $out_path || exit 1 done diff --git a/scripts/test.sh b/scripts/test.sh index 00e01e6..cba0681 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -1,22 +1,15 @@ #!/bin/bash # -# Run toybox tests. - -usage() -{ - cat <<EOF -Usage: - scripts/test.sh all - scripts/test.sh single COMMAND... - -Examples: - $ scripts/test.sh all # run tests for all commands - $ scripts/test.sh single grep sed # run tests for these two commands - - # Test 'grep' on the system, not toybox - $ TEST_HOST=1 scripts/test.sh commands grep -EOF -} +# Helper script for ./test.sh. This script assumes that the binaries are built +# and installed into $TOYBOX_TREE_DIR. +# +# Usage: +# scripts/test.sh all +# scripts/test.sh single COMMAND... +# +# Environment variables: +# TOYBOX_TREE_DIR: Must be set +# TEST_HOST: Test the command on the host instead of toybox [ -z "$TOPDIR" ] && TOPDIR="$(pwd)" @@ -42,7 +35,8 @@ setup_test_env() HOST_BIN_DATE=$(which date) HOST_BIN_HOSTNAME=$(which hostname) - PATH="$BIN_DIR:$PATH" # Make sure the tests can use toybox tools + # The tests should find toybox tools in their PATH first. + [ -z "$TEST_HOST" ] && PATH="$TOYBOX_TREE_DIR:$PATH" export LC_COLLATE=C # Library functions used by .test scripts, e.g. 'testing'. @@ -57,9 +51,6 @@ setup_test_env() # Run tests for specific commands. single() { - # Build individual binaries, e.g. generated/testdir/expr - [ -z "$TEST_HOST" ] && PREFIX=$BIN_DIR/ scripts/single.sh "$@" || exit 1 - setup_test_env for cmd in "$@" @@ -77,9 +68,6 @@ single() # Run tests for all commands. all() { - # Build a toybox binary and create symlinks to it. - [ -z "$TEST_HOST" ] && make install_flat PREFIX=$BIN_DIR/ || exit 1 - setup_test_env local failed_commands='' @@ -91,7 +79,7 @@ all() CMDNAME="${test_file##*/}" CMDNAME="${CMDNAME%.test}" - if [ -h $BIN_DIR/$CMDNAME ] || [ -n "$TEST_HOST" ] + if [ -h $TOYBOX_TREE_DIR/$CMDNAME ] || [ -n "$TEST_HOST" ] then local old_count=$FAILCOUNT cd_test_dir $CMDNAME @@ -125,10 +113,8 @@ all() } readonly TEST_ROOT=$TOPDIR/generated/test -readonly BIN_DIR=$TEST_ROOT/bin rm -rf $TEST_ROOT # clear out data from old runs -mkdir -p $BIN_DIR case $1 in single|all) diff --git a/test.sh b/test.sh new file mode 100755 index 0000000..f507cfb --- /dev/null +++ b/test.sh @@ -0,0 +1,164 @@ +#!/bin/bash +# +# Runs toybox tests, making sure to build the required binaries. + +set -o nounset +set -o pipefail +set -o errexit + +usage() { + cat <<EOF +Runs toybox tests, making sure to build the required binaries. + +Usage: + ./test.sh all [OPTION] # Run all tests + ./test.sh single [OPTION] COMMAND... # Run tests for the given commands + +Options: + -asan Run under AddressSanitizer + -msan Run under MemorySanitizer + -ubsan Run under UndefinedBehaviorSanitizer + +See http://clang.llvm.org/docs/index.html for details on these tools. + +Environment variables: + TEST_HOST: + Test the command on the host instead of toybox. Respected by + scripts/test.sh. + VERBOSE=1, VERBOSE=fail, DEBUG: + Show more test output. Respected by scripts/runtest.sh. + CLANG_DIR: + Directory of Clang pre-built binaries, available at + http://llvm.org/releases/download.html. You should set CLANG_DIR when + running with the sanitizers; otherwise it will use 'clang' in your PATH + (which is often old). + +Example: + $ export CLANG_DIR=~/install/clang+llvm-3.8.0-x86_64-linux-gnu-ubuntu-14.04 + $ ./test.sh all # Run all tests normally + $ ./test.sh single -asan grep sed # Run grep and sed tests under ASAN + +EOF + exit +} +# TODO: +# ./test.sh cov # run all tests and output coverage +# ./test.sh cov cat # coverage for cat only +# ./test.sh all -cov +# ./test.sh single -cov sed +# +# Where does the output go? +# coverage/ +# ALL.html +# sed.html + +readonly TOPDIR=${TOPDIR:-$PWD} +readonly CLANG_DIR=${CLANG_DIR:-} + +if [ -n "$CLANG_DIR" ] +then + # These are needed to show line numbers in stack traces. + sym=$CLANG_DIR/bin/llvm-symbolizer + export ASAN_SYMBOLIZER_PATH=$sym + export MSAN_SYMBOLIZER_PATH=$sym + export UBSAN_SYMBOLIZER_PATH=$sym +fi + +die() { echo "$@"; exit 1; } + +TOYBOX_BIN=toybox +SAN_FLAG= # Are we running under any Sanitizer? + +# Set globals if the arg looks like a flag. +process_flag() { + local flag=$1 + local ret=0 + case $flag in + -asan) + TOYBOX_BIN=toybox_asan + SAN_FLAG=$flag + ;; + -msan) + TOYBOX_BIN=toybox_msan + SAN_FLAG=$flag + ;; + -ubsan) + TOYBOX_BIN=toybox_ubsan + SAN_FLAG=$flag + export UBSAN_OPTIONS='print_stacktrace=1' + ;; + -*) + die "Invalid flag $flag" + ;; + *) + ret=1 # doesn't look like a flag + esac + return $ret +} + +# Print the toys that should be installed. +# +# TODO: This is a variant of logic in scripts/genconfig.sh, which has another +# variant in scripts/make.sh. scripts/genconfig.sh should probably also a +# simple text file of commands that everyone can use. +toys_to_link() { + grep 'TOY(.*)' toys/*/*.c | grep -v TOYFLAG_NOFORK | grep -v "0))" | \ + sed -rn 's/([^:]*):.*(OLD|NEW)TOY\( *([a-zA-Z][^,]*) *,.*/\3/p' | sort +} + +# Make a dir, linking every binary to the toybox binary. +make_toybox_tree() { + local tree_dir=$1 + local toybox_bin=$2 + + # Make there aren't old commands lying around. + rm -rf $tree_dir + mkdir -p $tree_dir + toys_to_link | xargs -I {} -- ln -s $toybox_bin $tree_dir/{} +} + +all() { + if [ $# -gt 0 ] + then + process_flag $1 && shift # set globals if it's a flag + fi + make $TOYBOX_BIN + local tree_dir=generated/tree/all$SAN_FLAG + make_toybox_tree $tree_dir ../../../$TOYBOX_BIN # 3 levels up + TOYBOX_TREE_DIR=$TOPDIR/$tree_dir scripts/test.sh all +} + +single() { + [ $# -eq 0 ] && die "At least one command is required." + process_flag $1 && shift # set globals if it's a flag + make $TOYBOX_BIN + + for cmd in "$@" + do + local tree_dir + [ -z "$SAN_FLAG" ] && + tree_dir=generated/tree/$cmd || tree_dir=generated/tree/all$SAN_FLAG + + make_toybox_tree $tree_dir ../../../$TOYBOX_BIN + + # Make the 'single' binary, and copy it over the symlink to toybox in the + # tree. + if [ -z "$SAN_FLAG" ] + then + make generated/single/$cmd + cp -v -f generated/single/$cmd $tree_dir + fi + TOYBOX_TREE_DIR=$TOPDIR/$tree_dir scripts/test.sh single $cmd + done +} + +[ $# -eq 0 ] && usage + +case $1 in + single|all) + "$@" + ;; + *) + usage + ;; +esac -- 1.9.1
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
