After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 724104 - Fix underlinking (floor/ceil) due to missing -lm
Fix underlinking (floor/ceil) due to missing -lm
Status: RESOLVED OBSOLETE
Product: glade
Classification: Applications
Component: general
3.16.x
Other Linux
: Normal normal
: ---
Assigned To: Glade 3 Maintainers
Glade 3 Maintainers
: 726622 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-02-11 08:30 UTC by Justin
Modified: 2018-03-26 15:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ld.gold patch (2.76 KB, patch)
2014-02-11 08:30 UTC, Justin
reviewed Details | Review
proposed patch, fixes underlinking with 3.18 (1.52 KB, patch)
2014-06-03 02:49 UTC, Alexandre Rostovtsev
none Details | Review

Description Justin 2014-02-11 08:30:59 UTC
Created attachment 268770 [details] [review]
ld.gold patch

Using ld.gold several underlinking where detected. Please find attach a patch against current HEAD to fix this issue.

libtool: link: x86_64-pc-linux-gnu-gcc -O2 -pipe -ftracer -march=native -frecord-gcc-switches -g -Wimplicit-function-declaration -Wl,-O1 -Wl,-O1 -Wl,--hash-style=gnu -Wl,--sort-common -o .libs/glade-previewer glade_previewer-glade-previewer.o -Wl,--export-dynamic -pthread  -Wl,--as-needed ./.libs/libgladeui-2.so -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -latk-1.0 -lcairo-gobject -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -lgmodule-2.0 -lglib-2.0 -lxml2 -pthread
./.libs/libgladeui-2.so: error: undefined reference to 'floor'
./.libs/libgladeui-2.so: error: undefined reference to 'ceil'
collect2: error: ld returned 1 exit status
Makefile:815: recipe for target 'glade-previewer' failed
Comment 1 Tristan Van Berkom 2014-02-11 09:04:19 UTC
Review of attachment 268770 [details] [review]:

Few comments:

For previewer: -lm belongs in LDFLAGS and not LDADD - it's possible that the OSX thing is wrong there too but let's not follow the same mistake

Same for the glade binary, -lm belongs in LDFLAGS not LDADD

For Gladeui_2_0_gir_LIBS, this should really be in an added -lm in Gladeui_2_0_gir_LDFLAGS, not 'm' in LIBS

For tests/Makefile.am, same treatment, and please specify this once in $(progs_libs) which will set each LDFLAGS appropriately (there's a reason why we defined that variable, so we dont have to do redundant flags in each separate test case).

Flags which access installed system libraries such as -L/usr/local/lib or or -lfoo (to find an installed libfoo.so in /usr/local/lib) - belong in LDFLAGS.

Specification of extra libtool archives built out of the local tree... such as specifying $(top_builddir)/gladeui/libgladeui-2.la .. belong in LIBS.
Comment 2 Justin 2014-02-11 09:07:41 UTC
libs never belong into LDFLAGS. LDFLAGS are instructions for the linker and not the libs.
Comment 3 Tristan Van Berkom 2014-02-11 09:26:13 UTC
(In reply to comment #2)
> libs never belong into LDFLAGS. LDFLAGS are instructions for the linker and not
> the libs.

I'm not seeing that in the automake manual.

For instance here:
http://www.gnu.org/software/automake/manual/automake.html#Libtool-Flags

Says:
  "the ‘library_LIBADD’ variable should be used to list extra 
   libtool objects (.lo files) or libtool libraries (.la) to add to library"

And basically that LDFLAGS it for extra linker instructions, I'm having a hard
time finding proper documentation around what _LIBS is really for.

In my experience, anything coming out of the PKG_CONFIG_CHECK() m4 macro
as DEP_LIBS - goes into LDFLAGS, and LDADD/LIBADD is for in-tree libtool
objects.

Can you point me to any authoritive documentation to back up your theory ?
Comment 5 Tristan Van Berkom 2014-02-11 09:49:48 UTC
From the gentoo wiki link:

  "The only case when this happens to be a problem is when the libraries 
   get fed into LDFLAGS variable (which is incorrect)."

I see this is the opinion of the gentoo wiki - but again - is there any
authoritive documentation from autotools which actually mentions this
is incorrect ?

The logic flowing from this would be that one would have to split out
the output from pkg-config so that the -L directives show up in LDFLAGS
and the -l directives show up in LIBS - which is just not how things work.
Comment 6 Justin 2014-02-11 10:04:34 UTC
Our leading developer, who pushes as-needed and wrote all those guides contacted the automake maintainers to update their documentation accordingly. So you can read it there in the future too.

Many projects including Fedora are using as-needed by default these days. All those project depend that the linker order is correct, meaning "-Wl,-as-needed OBJS LIBS". And this only works if you do not mix libs into LDFLAGS. Adding -L.. into libs doesn't harm. So pkg-config is probably not to doing the best job, but it doesn't break anything.
Comment 7 Tristan Van Berkom 2014-03-18 11:48:35 UTC
*** Bug 726622 has been marked as a duplicate of this bug. ***
Comment 8 Alexandre Rostovtsev 2014-06-03 02:47:42 UTC
The patch from comment #0 is addressing the issue from the wrong direction. It does unnecessary things, happens to fix underlinking with 3.16, but then breaks with 3.18:

/bin/sh ../libtool  --tag=CC   --mode=link x86_64-pc-linux-gnu-gcc  -march=native -O2 -pipe -frecord-gcc-switches   -Wl,--as-needed -Wl,-O1 -o glade-previewer glade_previewer-glade-previewer.o glade_previewer-glade-preview-window.o glade_previewer-glade-preview-template.o libgladeui-2.la   -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -latk-1.0 -lcairo-gobject -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -Wl,--export-dynamic -lgmodule-2.0 -pthread -lglib-2.0 -lxml2 
libtool: link: x86_64-pc-linux-gnu-gcc -march=native -O2 -pipe -frecord-gcc-switches -Wl,-O1 -o .libs/glade-previewer glade_previewer-glade-previewer.o glade_previewer-glade-preview-window.o glade_previewer-glade-preview-template.o -Wl,--export-dynamic -pthread  -Wl,--as-needed ./.libs/libgladeui-2.so -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -latk-1.0 -lcairo-gobject -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -lgmodule-2.0 -lglib-2.0 -lxml2 -pthread
./.libs/libgladeui-2.so: error: undefined reference to 'floor'
./.libs/libgladeui-2.so: error: undefined reference to 'ceil'
collect2: error: ld returned 1 exit status

The fundamental issue is that libgladeui *itself* directly uses floor() and ceil(). This means libgladeui must be linked with libm *and* everybody who links to libgladeui must automatically pull in libm to avoid undefined references - in other words, libgladeui-2.so needs libm in DT_NEEDED and libgladeui-2.la needs libm in dependency_libs. The patch proposed in comment #0 solved the first issue, but not the second.

The way to add libm to a library's DT_NEEDED and dependency_libs in automake syntax is to use LIBADD. So we need to append -lm in only one place: libgladeui_2_la_LIBADD.
Comment 9 Alexandre Rostovtsev 2014-06-03 02:49:45 UTC
Created attachment 277775 [details] [review]
proposed patch, fixes underlinking with 3.18
Comment 10 Juan Pablo Ugarte 2014-06-03 21:06:14 UTC
IIRC Gtk does link with -lm, So I wonder why that is not pulled automatically
Comment 11 Alexandre Rostovtsev 2014-06-04 01:19:03 UTC
(In reply to comment #10)
> IIRC Gtk does link with -lm, So I wonder why that is not pulled automatically

If you link to gtk, libm would be pulled in automatically at runtime (by ld.so).

However, at libgladeui's build time, the list of libraries is being obtained from $GTK_LIBS, and that is `pkg-config --libs gtk+-3.0` - and it doesn't have -lm. (If libgladeui had been using gtk+-3.0.la instead, -lm would probably have been there, but these days many distros avoid systemwide .la files because they cause upgrade dependency hell.)

What happens next depends on which ld you are using. The old ld.bfd linker will automagically pull in libm from gtk; but the new ld.gold won't do that unless you explicitly added -lm to ld's list of arguments, e.g. using LIBADD.
Comment 12 Pacho Ramos 2016-06-25 10:29:15 UTC
It looks like this is solved in 3.20.0
Comment 13 GNOME Infrastructure Team 2018-03-26 15:34:52 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glade/issues/149.