View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0009659CMakeCMakepublic2009-10-04 17:272009-10-08 11:58
ReporterModestas Vainius 
Assigned ToBrad King 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionCMake-2-8 
Target VersionFixed in Version 
Summary0009659: Executables should not be linked with shared library compilation flags
DescriptionI have cleaned up all Debian patches [1] for 2.8-rc2. It seems you applied some of the previous 2.6 patches to 2.8 but still a couple remain and I added a few more. In the perfect world I would like to get rid of them all. Patches have explanations in their headers so only short info about them below:

FindQt3.cmake.diff - Debian specific but harmless. You already applied similar patch to FindQt4.cmake.

desktop-remove-deprecated.diff - usage of deprecated field in .desktop file. Trivial.

executables-dont-need-fpic.diff - read the linked Debian bug report. I think you should apply the patch. Maybe you did fix it differently, I have not tested with unpatch cmake 2.8-rc2. If so, let me know.

fix_bashisms.diff - fixes portability problems when /bin/sh is not bash. Should be harmless.

kfreebsd-Platform.diff - you have some kFreeBSD support in Platform/kFreeBSD.cmake but it is incomplete because `uname -s` returns GNU/kFreeBSD, not just "kFreeBSD". So you should apply the first hunk (at least). The second hunk is not tested but kFreeBSD looks like Linux so it should benefit from stuff in Linux.cmake including Debian hacks.

manpage_friendly_docs.diff - renewed patch fixing new problems. Thanks for applying previous one. See patch header for details.

scripts-and-permissions.diff - a couple of problems with executable scripts in Modules/ and Templates/ and the way they get installed.

Sorry for "many bugs in one" report but opening that many reports for small issues would have been very inefficient for me.

1. http://git.debian.org/?p=collab-maint/cmake.git;a=tree;f=debian/patches;hb=refs/heads/experimental [^]
TagsNo tags attached.
Attached Filesdiff file icon executables-dont-need-fpic.diff [^] (1,244 bytes) 2009-10-06 04:26 [Show Content]

 Relationships
related to 0009985closedBrad King Linux.cmake should not hard-code -rdynamic 

  Notes
(0017940)
Bill Hoffman (manager)
2009-10-04 19:36

So the pic in exe comes from some compilers other than gnu that do template instantiation during linking and so actually compile code during the link. Why is this a problem?
(0017942)
Modestas Vainius (reporter)
2009-10-05 06:05

-fPIC enables Position Independent Code. Executables are not shared typically, their code does not need to be position independent contrary to shared libraries which are relocatable by definition. -fPIC is essential for shared libraries on GNU/Linux for all arches but i386.

Linking executables with -fPIC is just wrong and even considered a policy violation on Debian. I didn't get what you meant in your note, but in this case -fPIC comes from CMAKE_SHARED_LIBRARY_C_FLAGS in Modules/Platform/gcc.cmake and Modules/Platform/gcc.cmake. I don't know about other platforms (and don't care) but this is how it works on GNU/Linux and should be supported properly.

I have just confirmed the issue on unpatched cmake 2.8-rc2:

CMakeLists.txt
--------------
project(testproject C)
cmake_minimum_required(VERSION 2.6)
add_library(libtest SHARED test.c)
add_executable(testexe test.c)

Output
------
$ cmake .
.....
$ make VERBOSE=1
....
Linking C shared library liblibtest.so
/home/modax/src/cmake/cmake/builddir/bin/cmake -E cmake_link_script CMakeFiles/libtest.dir/link.txt --verbose=1
/mnt/sda2/usr/bin/gcc -fPIC -shared -Wl,-soname,liblibtest.so -o liblibtest.so CMakeFiles/libtest.dir/test.c.o

.... That's OK. -fPIC is here as it should be.

Linking C executable testexe
/home/modax/src/cmake/cmake/builddir/bin/cmake -E cmake_link_script CMakeFiles/testexe.dir/link.txt --verbose=1
/mnt/sda2/usr/bin/gcc -fPIC CMakeFiles/testexe.dir/test.c.o -o testexe -rdynamic

.... -fPIC is wrong here. I'm also told that -rdynamic here is *redundant* for most executables (check ld(1) docs for --export-dynamic). It increases executable size.

Clearing CMAKE_SHARED_LIBRARY_C_FLAGS in Modules/Platform/gcc.cmake drops -fPIC from both add_library() and add_executable(). This is wrong. -rdynamic is passed via CMAKE_SHARED_LIBRARY_LINK_C_FLAGS in Modules/Platform/Linux.cmake
(0017944)
Brad King (manager)
2009-10-05 09:56

In the bash-ism's patch, see this hunk:

----------------------------------------------------------------------------
diff --git a/Modules/CPack.RuntimeScript.in b/Modules/CPack.RuntimeScript.in
index eaecdd8..f27444f 100755
--- a/Modules/CPack.RuntimeScript.in
+++ b/Modules/CPack.RuntimeScript.in
@@ -3,10 +3,10 @@
 # Modified from: Aaron Voisine <aaron@voisine.org>
 
 CWD="`dirname \"$0\"`"
-TMP=/tmp/$UID/TemporaryItems
+TMP=/tmp/$(id -ru)/TemporaryItems
----------------------------------------------------------------------------

Isn't "$()" a bashism? Should it be

+TMP="/tmp/`id -ru`/TemporaryItems"

?
(0017945)
Brad King (manager)
2009-10-05 10:30
edited on: 2009-10-05 10:30

Applied FindQt3.cmake.diff:

  FindQt3: Prefer (moc|uic)-qt3 names over (moc|uic)
  /cvsroot/CMake/CMake/Modules/FindQt3.cmake,v <-- Modules/FindQt3.cmake
  new revision: 1.22; previous revision: 1.21

Applied desktop-remove-deprecated.diff:

  Remove old Encoding field from CMake.desktop
  /cvsroot/CMake/CMake/Source/QtDialog/CMake.desktop,v <-- Source/QtDialog/CMake.desktop
  new revision: 1.3; previous revision: 1.2

Applied kfreebsd-Platform.diff:

  Support GNU/kFreeBSD
  /cvsroot/CMake/CMake/Modules/CMakeDetermineSystem.cmake,v <-- Modules/CMakeDetermineSystem.cmake
  new revision: 1.30; previous revision: 1.29
  /cvsroot/CMake/CMake/Modules/Platform/kFreeBSD.cmake,v <-- Modules/Platform/kFreeBSD.cmake
  new revision: 1.5; previous revision: 1.4


Applied manpage_friendly_docs.diff:

  Fix module docs to be manpage (groff) friendly
  /cvsroot/CMake/CMake/Modules/FindBISON.cmake,v <-- Modules/FindBISON.cmake
  new revision: 1.4; previous revision: 1.3
  /cvsroot/CMake/CMake/Modules/FindFLEX.cmake,v <-- Modules/FindFLEX.cmake
  new revision: 1.3; previous revision: 1.2
  /cvsroot/CMake/CMake/Modules/FindProtobuf.cmake,v <-- Modules/FindProtobuf.cmake
  new revision: 1.6; previous revision: 1.5

Applied scripts-and-permissions.diff:

  Fix permsissions of installed SquishRunTestCase.sh
  /cvsroot/CMake/CMake/CMakeLists.txt,v <-- CMakeLists.txt
  new revision: 1.163; previous revision: 1.162

  Add '#!/bin/sh' to cygwin-package.sh
  /cvsroot/CMake/CMake/Templates/cygwin-package.sh.in,v <-- Templates/cygwin-package.sh.in
  new revision: 1.2; previous revision: 1.1

(0017952)
Modestas Vainius (reporter)
2009-10-05 12:39

Re bashisms patch.

Both are POSIX. See http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html [^]
2.6.3 Command Substitution
(0017955)
Brad King (manager)
2009-10-05 13:15

Applied fix_bashisms.diff:

  CPack: Fix bash-isms in launch script
  /cvsroot/CMake/CMake/Modules/CPack.RuntimeScript.in,v <-- Modules/CPack.RuntimeScript.in
  new revision: 1.6; previous revision: 1.5
(0017966)
Modestas Vainius (reporter)
2009-10-06 04:25

So I guess this bug could be renamed to "Executables should not be linked with flags of shared libraries". I also attach the description and the patch itself.

From: Ben Hutchings <ben@decadent.org.uk>
Subject: Do not use -fPIC when linking executables
 cmake includes ${CMAKE_SHARED_LIBRARY_C_FLAGS} in the command line to
 link an executable, and by default this is -fPIC. Either the use or
 the definition of this variable is wrong, because executables should
 not be linked with this option by default.
 .
 It's not entirely obvious how this variable gets into the command
 line, but you can verify that it does by changing its value to e.g. -D
 SHARED and running make VERBOSE=1.
 .
 Any special options needed for linking with shared libraries can be put
 in CMAKE_SHARED_LIBRARY_LINK_C_FLAGS.
Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=478404 [^]
(0017970)
Brad King (manager)
2009-10-06 10:17

I've traced down the history of the lines removed by the fPIC patch. They are the modern implementation of a change first made by this commit:

Author: Bill Hoffman <bill.hoffman@kitware.com>
Date: Thu Nov 14 01:14:05 2002 +0000

    add support for borland run time flag for shared builds

It's revision 1.30 of Source/cmLocalUnixMakefileGenerator.cxx:

http://www.cmake.org/cgi-bin/viewcvs.cgi/Source/cmLocalUnixMakefileGenerator.cxx?root=CMake&r1=1.29&r2=1.30 [^]

The relevant hunk looks like this:

     {
     rules.push_back(m_Makefile->GetDefinition("CMAKE_CXX_LINK_EXECUTABLE"));
     flags += this->GetSafeDefinition("CMAKE_CXX_FLAGS");
     flags += " ";
+ flags += this->GetSafeDefinition("CMAKE_SHARED_LIBRARY_CXX_FLAGS");
+ flags += " ";
     }

The goal was to build shared libraries with the Borland 5.5 free command line tools. The "-tWR" flag is needed for both compilation and linking to enable use of the dynamic runtime library.
(0018031)
Brad King (manager)
2009-10-08 11:57

I've committed totally new Borland support to avoid needing CMAKE_SHARED_LIBRARY_<lang>_FLAGS on the executable link line.

Split Borland compiler information files
/cvsroot/CMake/CMake/Modules/Platform/Windows-Borland-C.cmake,v <-- Modules/Platform/Windows-Borland-C.cmake
initial revision: 1.1
/cvsroot/CMake/CMake/Modules/Platform/Windows-Borland-CXX.cmake,v <-- Modules/Platform/Windows-Borland-CXX.cmake
initial revision: 1.1
/cvsroot/CMake/CMake/Modules/Platform/Windows-Borland.cmake,v <-- Modules/Platform/Windows-Borland.cmake
initial revision: 1.1
/cvsroot/CMake/CMake/Modules/Platform/Windows-bcc32.cmake,v <-- Modules/Platform/Windows-bcc32.cmake
new revision: delete; previous revision: 1.47
/cvsroot/CMake/CMake/Source/cmLocalGenerator.cxx,v <-- Source/cmLocalGenerator.cxx
new revision: 1.320; previous revision: 1.319
/cvsroot/CMake/CMake/Tests/CMakeLists.txt,v <-- Tests/CMakeLists.txt
new revision: 1.124; previous revision: 1.123
(0018032)
Brad King (manager)
2009-10-08 11:57

Now I've applied the -fPIC patch:

Do not use -fPIC to link executables
/cvsroot/CMake/CMake/Source/cmMakefileExecutableTargetGenerator.cxx,v <-- Source/cmMakefileExecutableTargetGenerator.cxx
new revision: 1.66; previous revision: 1.65

 Issue History
Date Modified Username Field Change
2009-10-04 17:27 Modestas Vainius New Issue
2009-10-04 19:35 Bill Hoffman Status new => assigned
2009-10-04 19:35 Bill Hoffman Assigned To => Brad King
2009-10-04 19:36 Bill Hoffman Note Added: 0017940
2009-10-05 06:05 Modestas Vainius Note Added: 0017942
2009-10-05 09:56 Brad King Note Added: 0017944
2009-10-05 10:30 Brad King Note Added: 0017945
2009-10-05 10:30 Brad King Note Edited: 0017945
2009-10-05 12:39 Modestas Vainius Note Added: 0017952
2009-10-05 13:15 Brad King Note Added: 0017955
2009-10-06 04:25 Modestas Vainius Note Added: 0017966
2009-10-06 04:26 Modestas Vainius File Added: executables-dont-need-fpic.diff
2009-10-06 08:34 Brad King Summary Debian patches against 2.8-rc2 => Executables should not be linked with shared library compilation flags
2009-10-06 10:17 Brad King Note Added: 0017970
2009-10-08 11:57 Brad King Note Added: 0018031
2009-10-08 11:57 Brad King Note Added: 0018032
2009-10-08 11:58 Brad King Status assigned => closed
2009-10-08 11:58 Brad King Resolution open => fixed
2010-01-12 17:10 Brad King Relationship added related to 0009985


Copyright © 2000 - 2018 MantisBT Team