tests/xenstore: link in librt

[PATCH] tests/xenstore: link in librt

Posted by Jan Beulich 2 years, 8 months ago

Old enough glibc has clock_gettime() in librt.so, hence the library
needs to be specified to the linker.

Fixes: 93c9edbef51b ("tests/xenstore: Rework Makefile")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Cc list based on the assumption that the XENSTORE section of
./MAINTAINERS probably ought to list the containing directory.

--- a/tools/tests/xenstore/Makefile
+++ b/tools/tests/xenstore/Makefile
@@ -31,6 +31,7 @@ CFLAGS += -Werror
 CFLAGS += $(CFLAGS_libxenstore)
 CFLAGS += $(APPEND_CFLAGS)
 
+LDFLAGS += -lrt
 LDFLAGS += $(LDLIBS_libxenstore)
 LDFLAGS += $(APPEND_LDFLAGS)

[PATCH] tests/xenstore: link in librt

Posted by Ian Jackson 2 years, 8 months ago

Jan Beulich writes ("[PATCH] tests/xenstore: link in librt"):
> Old enough glibc has clock_gettime() in librt.so, hence the library
> needs to be specified to the linker.
> 
> Fixes: 93c9edbef51b ("tests/xenstore: Rework Makefile")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Cc list based on the assumption that the XENSTORE section of
> ./MAINTAINERS probably ought to list the containing directory.
> 
> --- a/tools/tests/xenstore/Makefile
> +++ b/tools/tests/xenstore/Makefile
> @@ -31,6 +31,7 @@ CFLAGS += -Werror
>  CFLAGS += $(CFLAGS_libxenstore)
>  CFLAGS += $(APPEND_CFLAGS)
>  
> +LDFLAGS += -lrt

Don't this unconditionally is definitely not right.

How old a glibc are we talking about ?  (I looked at the minimum
versions listed in README and there's notthing about glibc; perhaps
should be.)

Ian.

Re: [PATCH] tests/xenstore: link in librt

Posted by Jan Beulich 2 years, 8 months ago

On 12.08.2021 13:24, Ian Jackson wrote:
> Jan Beulich writes ("[PATCH] tests/xenstore: link in librt"):
>> Old enough glibc has clock_gettime() in librt.so, hence the library
>> needs to be specified to the linker.
>>
>> Fixes: 93c9edbef51b ("tests/xenstore: Rework Makefile")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Cc list based on the assumption that the XENSTORE section of
>> ./MAINTAINERS probably ought to list the containing directory.
>>
>> --- a/tools/tests/xenstore/Makefile
>> +++ b/tools/tests/xenstore/Makefile
>> @@ -31,6 +31,7 @@ CFLAGS += -Werror
>>  CFLAGS += $(CFLAGS_libxenstore)
>>  CFLAGS += $(APPEND_CFLAGS)
>>  
>> +LDFLAGS += -lrt
> 
> Don't this unconditionally is definitely not right.

Assuming you meant "Doing this ..." - why? If the concern is that
librt.so may needlessly get recorded in a DT_NEEDED entry, then I
can replace the early addition with a late

LDFLAGS += -Wl,--as-needed -lc -lrt

one.

> How old a glibc are we talking about ?  (I looked at the minimum
> versions listed in README and there's notthing about glibc; perhaps
> should be.)

I've hit this with 2.11.3. I guess it has been made available by
libc.so from 2.17 onwards, considering the symbol version is
GLIBC_2.17.

Jan

Re: [PATCH] tests/xenstore: link in librt

Posted by Ian Jackson 2 years, 8 months ago

Jan Beulich writes ("Re: [PATCH] tests/xenstore: link in librt"):
> On 12.08.2021 13:24, Ian Jackson wrote:
> >> +LDFLAGS += -lrt
> > 
> > Don't this unconditionally is definitely not right.
> 
> Assuming you meant "Doing this ..." - why? If the concern is that
> librt.so may needlessly get recorded in a DT_NEEDED entry, then I
> can replace the early addition with a late
> 
> LDFLAGS += -Wl,--as-needed -lc -lrt
> 
> one.

librt might not exist at all on some platforms.

> > How old a glibc are we talking about ?  (I looked at the minimum
> > versions listed in README and there's notthing about glibc; perhaps
> > should be.)
> 
> I've hit this with 2.11.3. I guess it has been made available by
> libc.so from 2.17 onwards, considering the symbol version is
> GLIBC_2.17.

According to
  https://www.sourceware.org/glibc/wiki/Glibc%20Timeline
glibc 2.17 was released more than eight years ago on 2012-12-25.

Ian.

Re: [PATCH] tests/xenstore: link in librt

Posted by Jan Beulich 2 years, 8 months ago

On 13.08.2021 13:09, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH] tests/xenstore: link in librt"):
>> On 12.08.2021 13:24, Ian Jackson wrote:
>>>> +LDFLAGS += -lrt
>>>
>>> Don't this unconditionally is definitely not right.
>>
>> Assuming you meant "Doing this ..." - why? If the concern is that
>> librt.so may needlessly get recorded in a DT_NEEDED entry, then I
>> can replace the early addition with a late
>>
>> LDFLAGS += -Wl,--as-needed -lc -lrt
>>
>> one.
> 
> librt might not exist at all on some platforms.

Hmm, indeed. Do you have any suggestion then? Adding a ./configure
check is going to be beyond what I'd we willing to spend time on ...

Jan

Re: [PATCH] tests/xenstore: link in librt

Posted by Juergen Gross 2 years, 8 months ago

On 13.08.21 14:35, Jan Beulich wrote:
> On 13.08.2021 13:09, Ian Jackson wrote:
>> Jan Beulich writes ("Re: [PATCH] tests/xenstore: link in librt"):
>>> On 12.08.2021 13:24, Ian Jackson wrote:
>>>>> +LDFLAGS += -lrt
>>>>
>>>> Don't this unconditionally is definitely not right.
>>>
>>> Assuming you meant "Doing this ..." - why? If the concern is that
>>> librt.so may needlessly get recorded in a DT_NEEDED entry, then I
>>> can replace the early addition with a late
>>>
>>> LDFLAGS += -Wl,--as-needed -lc -lrt
>>>
>>> one.
>>
>> librt might not exist at all on some platforms.
> 
> Hmm, indeed. Do you have any suggestion then? Adding a ./configure
> check is going to be beyond what I'd we willing to spend time on ...

libxl is using (probably due to the same problem):

LDUSELIBS-$(CONFIG_Linux) += -lrt

So I guess the same construct is possible here, too.


Juergen