netdev - Re: [PATCH iproute2] build: Fix link errors on some systems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <875z4cwus8.fsf@nvidia.com>
Date:   Mon, 4 Jan 2021 17:07:03 +0100
From:   Petr Machata <petrm@...dia.com>
To:     Roi Dayan <roid@...dia.com>
CC:     <netdev@...r.kernel.org>, David Ahern <dsahern@...il.com>,
        Petr Machata <me@...chata.org>
Subject: Re: [PATCH iproute2] build: Fix link errors on some systems


Roi Dayan <roid@...dia.com> writes:

> Since moving get_rate() and get_size() from tc to lib, on some
> systems we fail to link because of missing the math lib.
> Move the link flag from tc makefile to the main makefile.

Hmm, yeah, it gets optimized out on x86-64. The issue is reproducible
on any platform with -O0.

> ../lib/libutil.a(utils.o): In function `get_rate':
> utils.c:(.text+0x10dc): undefined reference to `floor'
> ../lib/libutil.a(utils.o): In function `get_size':
> utils.c:(.text+0x1394): undefined reference to `floor'
> ../lib/libutil.a(json_print.o): In function `sprint_size':
> json_print.c:(.text+0x14c0): undefined reference to `rint'
> json_print.c:(.text+0x14f4): undefined reference to `rint'
> json_print.c:(.text+0x157c): undefined reference to `rint'
>
> Fixes: f3be0e6366ac ("lib: Move get_rate(), get_rate64() from tc here")
> Fixes: 44396bdfcc0a ("lib: Move get_size() from tc here")
> Fixes: adbe5de96662 ("lib: Move sprint_size() from tc here, add print_size()")
> Signed-off-by: Roi Dayan <roid@...dia.com>
> ---
>  Makefile    | 1 +
>  tc/Makefile | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index e64c65992585..2a604ec58905 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -59,6 +59,7 @@ SUBDIRS=lib ip tc bridge misc netem genl tipc devlink rdma dcb man
>  
>  LIBNETLINK=../lib/libutil.a ../lib/libnetlink.a
>  LDLIBS += $(LIBNETLINK)
> +LDFLAGS += -lm
>  
>  all: config.mk
>  	@set -e; \
> diff --git a/tc/Makefile b/tc/Makefile
> index 5a517af20b7c..8d91900716c1 100644
> --- a/tc/Makefile
> +++ b/tc/Makefile
> @@ -110,7 +110,7 @@ ifneq ($(TC_CONFIG_NO_XT),y)
>  endif
>  
>  TCOBJ += $(TCMODULES)
> -LDLIBS += -L. -lm
> +LDLIBS += -L.
>  
>  ifeq ($(SHARED_LIBS),y)
>  LDLIBS += -ldl

This will work, but it will give a libm dependency to all the tools.
util.c currently tries not to do that:

	/* emulate ceil() without having to bring-in -lm and always be >= 1 */
	*val = t;
	if (*val < t)
		*val += 1;

I think that just adding an unnecessary -lm is more of a tidiness issue
than anything else. One way to avoid it is to split the -lm deps out
from util.c / json_print.c to like util_math.c / json_print_math.c. That
way they will be in an .o of their own, and won't be linked in unless
the binary in question needs the code. Then the binaries that do call it
can keep on linking in -lm like they did so far.

Thoughts?