Ticket #540 (reopened bug)

Opened 5 years ago

Last modified 3 years ago

installed versions of dynext/*.so still link to -lparrot in build directory

Reported by: doughera Owned by: jkeenan
Priority: normal Milestone:
Component: install Version:
Severity: medium Keywords:
Cc: plobsing Language:
Patch status: Platform:

Description

The installed *.so files in lib/parrot/<version>/dynext/ are linked to the version of -lparrot in the build directory.

Before changing them to link to the -lparrot in the installed directory, however, it is worth asking whether they should be linked with -lparrot at all.

Once this is all settled, TT #476 should be revisited because it strips all rpath usage to work around this bug.

Attachments

glut_nci_thunks.diff Download (0.6 KB) - added by jkeenan 3 years ago.
No need for ALL_PARROT_LIBS here

Change History

follow-up: ↓ 2   Changed 5 years ago by heidnes

Another instance of this problem has been reported as a result of me packaging parrot version 1.0.0 for NetBSD's pkgsrc system. The problem report can be seen at  http://gnats.netbsd.org/41275/

Briefly summarized: libparrot_ldflags in the config points to the build directory. Additionally, on Solaris, RPATH in the installed binaries also point into the build directory, while it should point to the installed directory for the libraries.

in reply to: ↑ 1 ; follow-up: ↓ 3   Changed 5 years ago by doughera

Replying to heidnes:

Briefly summarized: libparrot_ldflags in the config points to the build directory.

I'm not sure if that's really a problem or not, since that variable isn't documented anywhere. However, it looks to me as if it's intended only to be used during the build process. The relevant variable for installed versions (also not documented anywhere) appears to be inst_libparrot_ldflags.

Additionally, on Solaris, RPATH in the installed binaries also point into the build directory, while it should point to the installed directory for the libraries.

Actually, what the report says is:

In addition under Solaris, the variable "rpath_blib" references the build directory as well, similar to libparrot_ldflags

This is definitely not a problem. The variable rpath_blib is actually documented to be relevant only for the build directory:

    # Set -rpath (or equivalent) for executables to find the
    # shared libparrot in the build directory.

So it retains a record of the build directory, but that's not important. It is not intended to be used in any way.

A related problem is that so many of these variables are undocumented, and what little documentation there is sprinkled throughout parrot's config/ directory doesn't get collected or installed anywhere. So someone working with an installed parrot has no easy way to determine whether such references are important or harmless.

in reply to: ↑ 2   Changed 5 years ago by coke

Replying to doughera:

Replying to heidnes:

Briefly summarized: libparrot_ldflags in the config points to the build directory.

I'm not sure if that's really a problem or not, since that variable isn't documented anywhere. However, it looks to me as if it's intended only to be used during the build process. The relevant variable for installed versions (also not documented anywhere) appears to be inst_libparrot_ldflags.

AHA!

Thank you. Somehow I missed this when trying to get partcl linking against an installed parrot, and was getting the build directory.

While surprising (I would think we would not want inst_ variants, but instead would want parrot_config to just use the installed versions in the regularly named variables), now I can almost get partcl building again. Thanks.

  Changed 5 years ago by doughera

Fixed for most Unix-like sytems with r39045 (see TT #698). Behavior on Win32 shouldn't have changed. A Win32 expert will need to consider carefully what the appropriate settings should be. I'll leave it open for now until that is settled. (AIX will also likely need separate treatment, but that should probably be a new ticket.)

follow-up: ↓ 6   Changed 4 years ago by jkeenan

  • status changed from new to assigned
  • owner set to jkeenan

If we have no further complaints about this problem, I'd like to close it in 7 days. Taking ticket solely for that purpose.

Thank you very much.

kid51

in reply to: ↑ 5   Changed 4 years ago by doughera

Replying to jkeenan:

If we have no further complaints about this problem, I'd like to close it in 7 days. Taking ticket solely for that purpose.

I never heard from a Win32 expert, so I don't know if this problem is still present on Win32.

  Changed 4 years ago by jkeenan

  • status changed from assigned to closed
  • resolution set to fixed

7 days ... and no objections heard. Closing ticket.

follow-up: ↓ 10   Changed 4 years ago by doughera

  • status changed from closed to reopened
  • resolution fixed deleted

This has partially resurfaced: extra_nci_thunks.so now links against -lparrot. I don't know why. However, it isn't installed, so perhaps it doesn't matter.

  Changed 4 years ago by jkeenan

  • status changed from reopened to new
  • owner jkeenan deleted

in reply to: ↑ 8 ; follow-up: ↓ 12   Changed 4 years ago by jkeenan

Replying to doughera:

This has partially resurfaced: extra_nci_thunks.so now links against -lparrot. I don't know why.

Is there any way other than examining the output of make to determine this? I.e., is there any command I could run against a file to see what it links against?

However, it isn't installed, so perhaps it doesn't matter.

Why would we create a .so file if it doesn't get installed?

Thank you very much.

kid51

follow-up: ↓ 13   Changed 4 years ago by jkeenan

Is runtime/parrot/dynext/libglutcb.so also affected in this way?

713-g++ -shared -O2 -L/usr/local/lib -fstack-protector -fPIC 
  -fstack-protector -L/usr/local/lib  \
714-    -o runtime/parrot/dynext/libglutcb.so src/glut_callbacks.o 
  src/glut_nci_thunks.o \
715:    -L"/home/jimk/gitwork/parrot/blib/lib" -lparrot -lpthread 
  -lm -L/usr/lib  -licuuc -licudata -lpthread -lm -lnsl -ldl 
  -lm -lcrypt -lutil -lpthread -lrt -lgmp -lreadline  -lffi   -lglut -lGLU -lGL

in reply to: ↑ 10   Changed 4 years ago by doughera

Replying to jkeenan:

Replying to doughera:

This has partially resurfaced: extra_nci_thunks.so now links against -lparrot. I don't know why.

Is there any way other than examining the output of make to determine this? I.e., is there any command I could run against a file to see what it links against?

On linux, at least,

ldd runtime/parrot/dynext/extra_nci_thunks.so

will list all libraries it links against.

However, it isn't installed, so perhaps it doesn't matter.

Why would we create a .so file if it doesn't get installed?

I have no idea.

in reply to: ↑ 11 ; follow-up: ↓ 14   Changed 4 years ago by jkeenan

Replying to jkeenan:

Is runtime/parrot/dynext/libglutcb.so also affected in this way?

$ ldd runtime/parrot/dynext/libglutcb.so |grep 'not found'
        libparrot.so.3.0.0 => not found
$ ldd runtime/parrot/dynext/extra_nci_thunks.so |grep 'not found'
        libparrot.so.3.0.0 => not found

Apparently so.

in reply to: ↑ 13   Changed 4 years ago by jkeenan

Replying to jkeenan:

Replying to jkeenan:

Is runtime/parrot/dynext/libglutcb.so also affected in this way?

{{{ $ ldd runtime/parrot/dynext/libglutcb.so |grep 'not found' libparrot.so.3.0.0 => not found $ ldd runtime/parrot/dynext/extra_nci_thunks.so |grep 'not found' libparrot.so.3.0.0 => not found }}} Apparently so.

If I apply this patch,

diff --git a/config/gen/makefiles/root.in b/config/gen/makefiles/root.in
index 4be06ad..8a9d3ce 100644
--- a/config/gen/makefiles/root.in
+++ b/config/gen/makefiles/root.in
@@ -2943,7 +2943,7 @@ src/glut_nci_thunks$(O) : $(PARROT_H_HEADERS) \
 $(LIBGLUTCB_SO): $(LIBPARROT) src/glut_callbacks$(O) src/glut_nci_thunks$(O)
        $(LD) $(LD_LOAD_FLAGS) $(LDFLAGS) \
     @ld_out@$@ src/glut_callbacks$(O) src/glut_nci_thunks$(O) \
-    $(ALL_PARROT_LIBS) @opengl_lib@
+    @opengl_lib@
 
 src/extra_nci_thunks.c : src/nci/extra_thunks.nci $(NCI_THUNK_GEN)
        $(NCI_THUNK_GEN) --dynext --no-warn-dups \
@@ -2959,8 +2959,7 @@ src/extra_nci_thunks$(O) : $(PARROT_H_HEADERS) src/extra_n
 
 $(EXTRANCITHUNKS_SO) : $(LIBPARROT) src/extra_nci_thunks$(O)
        $(LD) $(LD_LOAD_FLAGS) $(LDFLAGS) \
-    @ld_out@$@ src/extra_nci_thunks$(O) \
-    $(ALL_PARROT_LIBS)
+    @ld_out@$@ src/extra_nci_thunks$(O)
 
 # emacs etags
 # this needs exuberant-ctags

... I clear up the 'not found' entry and all tests pass, but in the case of extra_nci_thunks a lot of other linkages get wiped out.

$ ldd runtime/parrot/dynext/libglutcb.so 
        linux-gate.so.1 =>  (0xb7ff2000)
        libglut.so.3 => /usr/lib/libglut.so.3 (0xb7f97000)
        libGLU.so.1 => /usr/lib/libGLU.so.1 (0xb7f15000)
        libGL.so.1 => /usr/lib/libGL.so.1 (0xb7eb2000)
        libc.so.6 => /lib/libc.so.6 (0xb7d73000)
        libm.so.6 => /lib/libm.so.6 (0xb7d4d000)
        libXext.so.6 => /usr/lib/libXext.so.6 (0xb7d3f000)
        libX11.so.6 => /usr/lib/libX11.so.6 (0xb7c50000)
        libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0xb7b62000)
        libgcc_s.so.1 => /lib/libgcc_s.so.1 (0xb7b55000)
        libXxf86vm.so.1 => /usr/lib/libXxf86vm.so.1 (0xb7b4f000)
        libXdamage.so.1 => /usr/lib/libXdamage.so.1 (0xb7b4c000)
        libXfixes.so.3 => /usr/lib/libXfixes.so.3 (0xb7b47000)
        libpthread.so.0 => /lib/libpthread.so.0 (0xb7b2f000)
        libdl.so.2 => /lib/libdl.so.2 (0xb7b2b000)
        libdrm.so.2 => /usr/lib/libdrm.so.2 (0xb7b23000)
        /lib/ld-linux.so.2 (0x80000000)
        libXau.so.6 => /usr/lib/libXau.so.6 (0xb7b1f000)
        libxcb-xlib.so.0 => /usr/lib/libxcb-xlib.so.0 (0xb7b1d000)
        libxcb.so.1 => /usr/lib/libxcb.so.1 (0xb7b05000)
        libXdmcp.so.6 => /usr/lib/libXdmcp.so.6 (0xb7b00000)
$ ldd runtime/parrot/dynext/extra_nci_thunks.so 
        linux-gate.so.1 =>  (0xb7fc3000)
        libc.so.6 => /lib/libc.so.6 (0xb7e5a000)
        /lib/ld-linux.so.2 (0x80000000)

Perhaps extra_nci_thunks.so doesn't need all those linkages.

follow-up: ↓ 16   Changed 4 years ago by jkeenan

  • cc plobsing added

plobsing,

Could you comment on the issues raised in comments 10+ in this ticket? Can we eliminate the linkage of extra_nci_thunks against libparrot?

Thank you very much.

kid51

in reply to: ↑ 15 ; follow-up: ↓ 17   Changed 4 years ago by plobsing

Replying to jkeenan:

plobsing, Could you comment on the issues raised in comments 10+ in this ticket? Can we eliminate the linkage of extra_nci_thunks against libparrot?

extra_nci_thunks.c includes function calls to Parrot_pcc_fill_params_from_c_args(), which is part of libparrot. These calls are essential to the operation of this library, therefore it must be linked against libparrot to operate correctly.

As to the purpose of this library, it is the repetition of extra_thunks.nci as a dynlib, used to demonstrate the viability of using dynlibs to provide additional nci thunks. One can configure --without-extra-nci-thunks, load this library, and run tests that depend on these thunks (eg: t/pmc/nci.t) which would normally fail with this configuration. I intended the configure-without-extra/load-as-separate-library as the default approach, however, this approach never caught on (I've heard complaints that "you can't use anything but the built-in signatures" months after this was no longer the case). I think it still has value, although I'm a bit burned out on the whole NCI thing.

Thank you very much. kid51

in reply to: ↑ 16 ; follow-up: ↓ 18   Changed 4 years ago by doughera

Replying to plobsing:

extra_nci_thunks.c includes function calls to Parrot_pcc_fill_params_from_c_args(), which is part of libparrot. These calls are essential to the operation of this library, therefore it must be linked against libparrot to operate correctly.

I don't understand this. The same could be said about other items in dynext/, but they aren't linked directly against libparrot. I don't see why extra_nci_thunks.so needs special treatment.

in reply to: ↑ 17 ; follow-up: ↓ 19   Changed 4 years ago by plobsing

Replying to doughera:

Replying to plobsing:

extra_nci_thunks.c includes function calls to Parrot_pcc_fill_params_from_c_args(), which is part of libparrot. These calls are essential to the operation of this library, therefore it must be linked against libparrot to operate correctly.

I don't understand this. The same could be said about other items in dynext/, but they aren't linked directly against libparrot. I don't see why extra_nci_thunks.so needs special treatment.

Perhaps it is based on a misunderstanding of dynamic linking, but my preference is a direct and explicit link for all parrot-expecting dynext libraries. The simple reason is that an implicit "whatever loads me will have these symbols defined" approach breaks down occasionally. For example, blizkost had (and subsequently worked around at the cost of developer time) issues with XS libraries because these libraries made exactly that assumption, and it proved not to be true.

in reply to: ↑ 18 ; follow-up: ↓ 20   Changed 3 years ago by doughera

Replying to plobsing:

Perhaps it is based on a misunderstanding of dynamic linking, but my preference is a direct and explicit link for all parrot-expecting dynext libraries. The simple reason is that an implicit "whatever loads me will have these symbols defined" approach breaks down occasionally.

I understand that as a general idea, but in this specific case, I find it difficult to imagine a useful program trying to load extra_nci_thunks.so that hasn't already loaded libparrot.

Moreover, I still don't see any reason for treating some of the dynext/*.so files differently from the others. Whatever is done for one should probably be done for all.

If the dynext/*.so files *are* to be linked against -lparrot, then the appropriate rpath should be set so that the correct version of -lparrot is found. As a practical matter, this likely means that two copies of each have to be built: Those built to be used in the build directory need to be linked with rpath_blib, while those that are to be installed need to be linked with rpath_lib.

in reply to: ↑ 19   Changed 3 years ago by jkeenan

Replying to doughera:

in this specific case, I find it difficult to imagine a useful program trying to load extra_nci_thunks.so that hasn't already loaded libparrot. Moreover, I still don't see any reason for treating some of the dynext/*.so files differently from the others. Whatever is done for one should probably be done for all.

plobsing,

Can you comment on this?

If the dynext/*.so files *are* to be linked against -lparrot, then the appropriate rpath should be set so that the correct version of -lparrot is found. As a practical matter, this likely means that two copies of each have to be built: Those built to be used in the build directory need to be linked with rpath_blib, while those that are to be installed need to be linked with rpath_lib.

It seems to me that either we don't link dynext/*.so files against -lparrot, or else we undertake the work described by doughera with respect to rpath_blib or rpath_lib. The former seems simpler.

kid51

  Changed 3 years ago by jkeenan

I no longer find runtime/parrot/dynext/extra_nci_thunks.so.

[parrot] 522 $ find . -type f -name '*extra-nci-thunks*'
[parrot] 523 $ find . -type f -name '*extra_nci_thunks*'

Does that render the issues raised in this ticket moot? If so, can we close this ticket?

And what does that mean for other places where 'extra(_|-)nci(_|-) appears in our code?

$ ack -il --match 'extra(_|-)nci(_|-)thunks' *
MANIFEST.SKIP
config/auto/warnings.pm
config/init/defaults.pm
lib/Parrot/Configure/Options/Conf/Shared.pm
lib/Parrot/Configure/Options/Conf.pm
src/global_setup.c
t/steps/init/defaults-01.t

Thank you very much.

kid51

Changed 3 years ago by jkeenan

No need for ALL_PARROT_LIBS here

  Changed 3 years ago by jkeenan

  • status changed from new to assigned
  • owner set to jkeenan

The patch attached, glut_nci_thunks_diff, eliminates the last case where a linkage was not found.

$ ldd runtime/parrot/dynext/*.so | grep 'not found'
$ 

I have tested in thru make test and make fulltest on linux/i386. I propose that we apply the patch and close this ticket.

Thank you very much.

kid51

  Changed 3 years ago by cotto

If winxed's opengl demo works with the attached patch, let's apply it and close this ticket.

follow-up: ↓ 25   Changed 3 years ago by NotFound

I've tested in amd64 with the patch applied and all winxed examples using opengl works. I'm using libffi in this machine, don't know if that is relevant for the issue.

in reply to: ↑ 24   Changed 3 years ago by jkeenan

  • status changed from assigned to closed
  • resolution set to fixed

Replying to NotFound:

I've tested in amd64 with the patch applied and all winxed examples using opengl works. I'm using libffi in this machine, don't know if that is relevant for the issue.

Thanks for that test. Thanks to all who contributed. Closing ticket.

kid51

  Changed 3 years ago by jkeenan

  • status changed from closed to reopened
  • resolution fixed deleted

  Changed 3 years ago by jkeenan

Sadly, I had to reopen this ticket due to a  build error on Cygwin reported by taptinder -- a platform to which I don't have access.

Note: See TracTickets for help on using tickets.