[oe] [meta-oe] [PATCHv3] lmbench: Fix/clean-up the build for lmbench

Randy MacLeod randy.macleod at windriver.com
Fri Oct 18 19:49:51 UTC 2019


On 10/18/19 7:42 AM, marek.bykowski at gmail.com wrote:
> Fix/clean up a couple of things:
> - if lmbench doesn't use a return from a function attributed to warn_unused_result
>    suppress the warning/s
> - fix buffer overflow
> - define feature_test_macro_GNU_SOURCE compiling in sched_{get,set}affinity()
> - go strict for a pre-build check if sched_{get,set}affinity() from glibc builds
>    cleanly. If it is not compile it out.
> 
> Signed-off-by: Marek Bykowski<marek.bykowski at gmail.com>
> Reviewed-off-by: Marcin Lapaj<marcin.lapaj at tieto.com>
> ---
> Changelog V2->V3:
> - primary I have made it strict in a pre-build check if sched_{get,set}affinity()
>    do not throw any warnings. If they do there is a possibility of a mismatch
>    between the routines signatures in the app and their definitions in the glibc
>    and, as a result, decided on skipping the affinity all together. Anyway I don't
>    think affinity is used that often (if at all) in lmbench. It is better-off
>    to script it around and use "taskset" to control the CPU's affinity
> - additionally I have tested building it against x86 and aarch64 toolchains
> ---
>   ...h-Fix-clean-up-the-build-for-lmbench.patch | 475 ++++++++++++++++++
>   .../lmbench/lmbench_3.0-a9.bb                 |   1 +
>   2 files changed, 476 insertions(+)
>   create mode 100644 meta-oe/recipes-benchmark/lmbench/lmbench-3.0-a9/0001-lmbench-Fix-clean-up-the-build-for-lmbench.patch
> 
> diff --git a/meta-oe/recipes-benchmark/lmbench/lmbench-3.0-a9/0001-lmbench-Fix-clean-up-the-build-for-lmbench.patch b/meta-oe/recipes-benchmark/lmbench/lmbench-3.0-a9/0001-lmbench-Fix-clean-up-the-build-for-lmbench.patch
> new file mode 100644
> index 000000000..ba650040e
> --- /dev/null
> +++ b/meta-oe/recipes-benchmark/lmbench/lmbench-3.0-a9/0001-lmbench-Fix-clean-up-the-build-for-lmbench.patch
> @@ -0,0 +1,475 @@
> +lmbench: Fix/clean-up the build for lmbench
> +
> +Fix/clean up a coule of things:
> +- if lmbench doesn't use a return from a function attributed to warn_unused_result
> +  supress the warning/s
> +- fix buffer overflow
> +- define feature_test_macro_GNU_SOURCE compiling in sched_{get,set}affinity()
> +- go strict for a pre-build check if sched_{get,set}affinity() from glibc builds
> +  cleanly. If it is not compile it out.
> +
> +Signed-off-by: Marek Bykowski<marek.bykowski at gmail.com>
> +Reviewed-off-by: Marcin Lapaj<marcin.lapaj at tieto.com>
> +
> +---
> + scripts/build          | 24 +++++++++++++++---------
> + src/bw_tcp.c           |  2 +-
> + src/bw_unix.c          |  4 ++--
> + src/hello.c            |  2 +-
> + src/lat_connect.c      |  2 +-
> + src/lat_fcntl.c        |  4 ++--
> + src/lat_fs.c           |  2 +-
> + src/lat_http.c         |  6 +++---
> + src/lat_proc.c         |  2 +-
> + src/lat_select.c       |  2 +-
> + src/lat_tcp.c          |  8 ++++----
> + src/lat_unix.c         |  2 +-
> + src/lat_unix_connect.c |  4 ++--
> + src/lib_sched.c        |  7 +++++--
> + src/lib_timing.c       | 18 +++++++++---------
> + src/lmhttp.c           | 26 +++++++++++++++++---------
> + src/memsize.c          |  4 ++--
> + 17 files changed, 68 insertions(+), 51 deletions(-)
> +
> +diff --git a/scripts/build b/scripts/build
> +index 3786741..c0930d6 100755
> +--- a/scripts/build
> ++++ b/scripts/build
> +@@ -33,7 +33,7 @@ else
> + 	fi
> + fi
> + rm -f ${BASE}$$ ${BASE}$$.o ${BASE}$$.c
> +-	
> ++
> + # check for IA64 HP-UX w/ HP's ANSI compiler; may need pointer swizzling
> + arch=`echo $OS | awk -F- '{print $1;}'`
> + if [ "X$CC" = "Xcc" -a "X$arch" = "Xia64" ]
> +@@ -272,14 +272,20 @@ ${CC} ${CFLAGS} -o ${BASE}$$ ${BASE}$$.c ${LDLIBS} 1>${NULL} 2>${NULL} \
> + 	&& CFLAGS="${CFLAGS} -DHAVE_BINDPROCESSOR=1";
> + rm -f ${BASE}$$ ${BASE}$$.o ${BASE}$$.c
> +
> +-# check that we have sched_setaffinity
> +-echo "#include <stdlib.h>" > ${BASE}$$.c
> +-echo "#include <unistd.h>" >> ${BASE}$$.c
> +-echo "#include <sched.h>" >> ${BASE}$$.c
> +-echo "main() { unsigned long mask = 1; return sched_setaffinity(0, sizeof(unsigned long), &mask); }" >> ${BASE}$$.c
> +-${CC} ${CFLAGS} -o ${BASE}$$ ${BASE}$$.c ${LDLIBS} 1>${NULL} 2>${NULL} \
> +-	&& CFLAGS="${CFLAGS} -DHAVE_SCHED_SETAFFINITY=1";
> +-rm -f ${BASE}$$ ${BASE}$$.o ${BASE}$$.c
> ++# check that we have sched_setaffinity and it builds cleanly
> ++cat << '_ACEOF' > ${BASE}$$.c
> ++#define _GNU_SOURCE
> ++#include <stdlib.h>
> ++#include <unistd.h>
> ++#include <sched.h>
> ++int main() {
> ++cpu_set_t set; CPU_ZERO(&set); CPU_SET(0, &set);
> ++if (sched_setaffinity(0, sizeof(set), &set) == -1) exit(EXIT_FAILURE); exit(EXIT_SUCCESS);
> ++}
> ++_ACEOF
> ++${CC} ${CFLAGS} -o ${BASE}$$ ${BASE}$$.c ${LDLIBS} 1>${NULL} 2>${BASE}$$_err
> ++test -s ${BASE}$$_err || CFLAGS="${CFLAGS} -DHAVE_SCHED_SETAFFINITY=1"
> ++rm -f ${BASE}$$_err ${BASE}$$ ${BASE}$$.o ${BASE}$$.c
> +
> +
> + if [ ! -d ${BINDIR} ]; then mkdir -p ${BINDIR}; fi
> +diff --git a/src/bw_tcp.c b/src/bw_tcp.c
> +index cc27098..bf76fac 100644
> +--- a/src/bw_tcp.c
> ++++ b/src/bw_tcp.c
> +@@ -60,7 +60,7 @@ main(int ac, char **av)
> + 		{
> + 			int	conn;
> + 			conn = tcp_connect(optarg, TCP_DATA, SOCKOPT_NONE);
> +-			write(conn, "0", 1);
> ++			(void) !write(conn, "0", 1);
> + 			exit(0);
> + 		}
> + 		case 'm':
> +diff --git a/src/bw_unix.c b/src/bw_unix.c
> +index ad71157..493db7c 100644
> +--- a/src/bw_unix.c
> ++++ b/src/bw_unix.c
> +@@ -97,7 +97,7 @@ reader(iter_t iterations, void* cookie)
> + 	size_t	todo = state->bytes;
> +
> + 	while (iterations-- > 0) {
> +-		write(state->control[1], &todo, sizeof(todo));
> ++		(void) !write(state->control[1], &todo, sizeof(todo));
> + 		for (done = 0; done < todo; done += n) {
> + 			if ((n = read(state->pipes[0], state->buf, state->xfer)) <= 0) {
> + 				/* error! */
> +@@ -115,7 +115,7 @@ writer(int controlfd, int writefd, char* buf, void* cookie)
> + 	struct _state* state = (struct _state*)cookie;
> +
> + 	for ( ;; ) {
> +-		read(controlfd, &todo, sizeof(todo));
> ++		(void) !read(controlfd, &todo, sizeof(todo));
> + 		for (done = 0; done < todo; done += n) {
> + #ifdef TOUCH
> + 			touch(buf, XFERSIZE);
> +diff --git a/src/hello.c b/src/hello.c
> +index 15a2493..0f7f237 100644
> +--- a/src/hello.c
> ++++ b/src/hello.c
> +@@ -3,6 +3,6 @@
> + int
> + main()
> + {
> +-	write(1, "Hello world\n", 12);
> ++	(void) !write(1, "Hello world\n", 12);


Hi Merek,

What's the "!" for ? It doesn't seem to be essential.

In unistd.h:
extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur;

and in cdefs.h:
---------------
/* If fortification mode, we warn about unused results of certain
    function calls which can lead to problems.  */
#if __GNUC_PREREQ (3,4)
# define __attribute_warn_unused_result__ \
    __attribute__ ((__warn_unused_result__))
# if __USE_FORTIFY_LEVEL > 0
#  define __wur __attribute_warn_unused_result__
# endif
#else
# define __attribute_warn_unused_result__ /* empty */
#endif
#ifndef __wur
# define __wur /* Ignore */
#endif
---------------

so did you consider just making a compiler specific fix/hack
for gcc/clang such as:
    #define __wur /* Ignore */
You could do that in each file that needs it.
While that's still ugly, it is at least more localized.


Also, did you confirm that this change doesn't affect the
resulting binary code? I expect that it won't but it's a
benchmark so it's worth confirming.


Finally, lmbench hasn't released a new tarball on sf.net in 5 years:
    https://sourceforge.net/projects/lmbench/
There's a new somewhat active version of lmbench:
    https://github.com/intel/lmbench
Should we use that version?

Sorry to leave this until v3 but the:
    (void) !foo()
syntax triggered me! ;-)

-- 
# Randy MacLeod
# Wind River Linux


More information about the Openembedded-devel mailing list