[LTP] [RFC] [PATCH] lib: Fix undefined reference to `mq_open' build failures

[LTP] [RFC] [PATCH] lib: Fix undefined reference to `mq_open' build failures

[Cyril Hrubis]

It appears that since the addition of the tst_safe_posix_ipc.c to the
test library random testcases (mostly ltp-aiodio seems to be triggering
the issue) started to fail on linking with missing reference to mq_open.

The problem is that -lrt is needed for mq_open() so this commit adds a
weak stub symbol that is used as fallback when we are compiling without
-lrt.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 lib/tst_safe_posix_ipc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/tst_safe_posix_ipc.c b/lib/tst_safe_posix_ipc.c
index 7142a25..4c617c8 100644
--- a/lib/tst_safe_posix_ipc.c
+++ b/lib/tst_safe_posix_ipc.c
@@ -22,6 +22,13 @@
 #include "tst_test.h"
 #include "tst_safe_posix_ipc.h"
 
+mqd_t __attribute__((weak)) mq_open(const char *name __attribute__((unused)),
+				    int oflag __attribute__((unused)), ...)
+{
+	tst_brk(TBROK, "mq_open() stub called!");
+	return 0;
+}
+
 int safe_mq_open(const char *file, const int lineno, const char *pathname,
 	int oflags, ...)
 {
-- 
2.10.2

[LTP] [RFC] [PATCH] lib: Fix undefined reference to `mq_open' build failures

[Richard Palethorpe]

Hi Metan,

On Wed, 29 Mar 2017 18:50:08 +0200
"Cyril Hrubis" <chrubis@suse.cz> wrote:

> It appears that since the addition of the tst_safe_posix_ipc.c to the
> test library random testcases (mostly ltp-aiodio seems to be triggering
> the issue) started to fail on linking with missing reference to mq_open.
> 
> The problem is that -lrt is needed for mq_open() so this commit adds a
> weak stub symbol that is used as fallback when we are compiling without
> -lrt.
> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  lib/tst_safe_posix_ipc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/lib/tst_safe_posix_ipc.c b/lib/tst_safe_posix_ipc.c
> index 7142a25..4c617c8 100644
> --- a/lib/tst_safe_posix_ipc.c
> +++ b/lib/tst_safe_posix_ipc.c
> @@ -22,6 +22,13 @@
>  #include "tst_test.h"
>  #include "tst_safe_posix_ipc.h"
>  
> +mqd_t __attribute__((weak)) mq_open(const char *name __attribute__((unused)),
> +				    int oflag __attribute__((unused)), ...)
> +{
> +	tst_brk(TBROK, "mq_open() stub called!");

Maybe expand this to "mq_open() stub called! Add -lrt to linker flags".

If we start stubbing a lot of functions then we could have a TST_STUB macro
which takes the function name, arguments and linker switch (e.g. -lrt) and
produces a stub.

> +	return 0;
> +}
> +
>  int safe_mq_open(const char *file, const int lineno, const char *pathname,
>  	int oflags, ...)
>  {

I ran into this problem and the stub fixed it. I wonder why a test which never
calls SAFE_MQ_OPEN triggers it and others do not and why it does not happen
consistently. Perhaps the linker tries to do some relocation at a stage when
the library is not available and whether it tries to do the relocation is
effected by whatever libraries are available in the environment. I can't think
why else it would only try to resolve it some of the time?

Anyway, I have tried it on gcc6, clang and gcc4 without any problems. It seems
like a good approach.

Thank you,
Richard.

[LTP] [RFC] [PATCH] lib: Fix undefined reference to `mq_open' build failures

[Jan Stancek]

----- Original Message -----
> 
> Anyway, I have tried it on gcc6, clang and gcc4 without any problems. It
> seems
> like a good approach.

Also looks good on RHEL5.6/6.0/7.3.

Regards,
Jan

[LTP] [RFC] [PATCH] lib: Fix undefined reference to `mq_open' build failures

[Cyril Hrubis]

Hi!
> > +mqd_t __attribute__((weak)) mq_open(const char *name __attribute__((unused)),
> > +				    int oflag __attribute__((unused)), ...)
> > +{
> > +	tst_brk(TBROK, "mq_open() stub called!");
> 
> Maybe expand this to "mq_open() stub called! Add -lrt to linker flags".

Good idea, I will add a hint that -lr is probably missing in LDFLAGS to
the message.

> If we start stubbing a lot of functions then we could have a TST_STUB macro
> which takes the function name, arguments and linker switch (e.g. -lrt) and
> produces a stub.

That could end up quite complicated fast. For instance adding
__attribute__((unused)) to each of the parametes would either need
different version of the macro for each number of arguments or load of
macro trickery...

Let's keep it simple for now.

> > +	return 0;
> > +}
> > +
> >  int safe_mq_open(const char *file, const int lineno, const char *pathname,
> >  	int oflags, ...)
> >  {
> 
> I ran into this problem and the stub fixed it. I wonder why a test which never
> calls SAFE_MQ_OPEN triggers it and others do not and why it does not happen
> consistently. Perhaps the linker tries to do some relocation at a stage when
> the library is not available and whether it tries to do the relocation is
> effected by whatever libraries are available in the environment. I can't think
> why else it would only try to resolve it some of the time?

I do not understand why the problem manifests only rarely either, but I
fear that understanding it would require a few days of research...

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [RFC] [PATCH] lib: Fix undefined reference to `mq_open' build failures

[Cyril Hrubis]

Hi!
I've pushed the patch with acks from Ritchie and Jan.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [RFC] [PATCH] lib: Fix undefined reference to `mq_open' build failures

[Cyril Hrubis]

Hi!
And unfortunatelly this approach does not seem to work for static
linking :-(

Seems I misunderstand how weak symbols are handled, since if the weak
symbol is linked into the binary the linker does not even try to
resolve it hence the strong symbol in librt.so is ignored.

Basically what we wanted to have is the weak_import attribute that could
be used on apple OSX.

Anyway possible solutions I can come up with:


* Switch to dynamic linking for libltp

  - may be worth the saving some megabytes on file size

  - would require the dynamic library to be put in the correct path or
    messing with LD_LIBRARY_PATH though


* Getting pointer to the mq_open() function via
  dlysm(RTLD_DEFAULT, "mq_open") and TBROK if we get NULL

  - this requires the test linked with -ldl though
    so this is not helping much


* Splitting the libltp.a into libltp.a and libltprt.a

  - this is not elegant, but bullet-proof


* Hacking around build system and link the stub
  only when -lrt is not in CFLAGS

  - this is hackery, but would probably work fine


* Anything else?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [RFC] [PATCH] lib: Fix undefined reference to `mq_open' build failures

[Jan Stancek]

----- Original Message -----
> Hi!
> And unfortunatelly this approach does not seem to work for static
> linking :-(
> 
> Seems I misunderstand how weak symbols are handled, since if the weak
> symbol is linked into the binary the linker does not even try to
> resolve it hence the strong symbol in librt.so is ignored.
> 
> Basically what we wanted to have is the weak_import attribute that could
> be used on apple OSX.
> 
> Anyway possible solutions I can come up with:
> 
> 
> * Switch to dynamic linking for libltp
> 
>   - may be worth the saving some megabytes on file size
> 
>   - would require the dynamic library to be put in the correct path or
>     messing with LD_LIBRARY_PATH though

I'd avoid this, as it makes it more difficult to run tests in-tree.

> 
> 
> * Getting pointer to the mq_open() function via
>   dlysm(RTLD_DEFAULT, "mq_open") and TBROK if we get NULL
> 
>   - this requires the test linked with -ldl though
>     so this is not helping much
> 
> 
> * Splitting the libltp.a into libltp.a and libltprt.a
> 
>   - this is not elegant, but bullet-proof
> 
> 
> * Hacking around build system and link the stub
>   only when -lrt is not in CFLAGS
> 
>   - this is hackery, but would probably work fine
> 
> 
> * Anything else?

Move safe_mq_open to header?

> 
> --
> Cyril Hrubis
> chrubis@suse.cz
> 
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
> 

[LTP] [RFC] [PATCH] lib: Fix undefined reference to `mq_open' build failures

[Cyril Hrubis]

Hi!
> > * Anything else?
> 
> Move safe_mq_open to header?

I guess that I missed the obviously easiest solution. I will commit a
change that moves the functio to the header.

-- 
Cyril Hrubis
chrubis@suse.cz