Bug 489046

Unhiding this bug. It makes no sense to keep this restricted to Fedora contributors as all the information is also public in the review request.

From the Zorba review request:

Comment #24 From  Kevin Kofler 2009-03-07 18:04:32 EDT
I suspect this may be the same bug which triggers the random crashes during
kdepimlibs builds (which vanish when we try to debug them). See bug 475876 for
that one.

Comment #25 From  Kevin Kofler  2009-03-07 18:28:58 EDT
Hmmm, this is probably different from the other bug. The problem here is that
${CMAKE_CURRENT_LIST_FILE} is invalid for some reason. We need to figure out
why. The CMake call stack could help, but what would be even more useful would
be running Valgrind over cmake (with cmake-debuginfo installed) to see where
the memory corruption or the invalid pointer happens.

Comment #26 From  Kevin Kofler  2009-03-07 18:47:21 EDT
Actually I think I already know what's wrong. This is how
CMAKE_CURRENT_LIST_FILE is implemented (unless it changed since):
http://www.cmake.org/Bug/file_download.php?file_id=600&type=bug

The problem here is that it saves the value into a std::string, then restores
it from that string's c_str. That pointer is no longer valid once the
std::string goes out of scope, which is soon afterwards. The value probably
needs to be saved into a const char * instead.

Comment #27 From  Kevin Kofler  2009-03-07 19:00:23 EDT
Actually, the definitions are stored in a map using std::string, so this can't
be it (AddDefinition copies the passed const char * into a new std::string, so
when the pointer goes out of scope, it's supposed to be copied already).

I think we really need a Valgrind log.

Comment #28 From  Kevin Kofler  2009-03-08 21:51:36 EDT
Any chance we can get a log from running cmake in Valgrind, if possible with
cmake-debuginfo installed?

The problem is related, however, because if  cmMakefile::ReadListFile(...)  is patched to use dynamically allocated copies of the C-strings (e.g. instead of assigning filenametoread = external), the problem goes away. Then, instead of the error seen in bug 457160 comment 13 (a result of a freed/corrupted RelativePath value) I get this:

CMake Error at cmake_modules/AddSrcSubfolder.cmake:18 (INCLUDE):
  include could not find load file:

    ../cmake_modules/zorbaerrors/CMakeLists.txt
Call Stack (most recent call first):
  src/CMakeLists.txt:54 (ADD_SRC_SUBFOLDER)

CMake Error at cmake_modules/AddSrcSubfolder.cmake:18 (INCLUDE):
  include could not find load file:

    ../cmake_modules/zorbautils/CMakeLists.txt
Call Stack (most recent call first):
  src/CMakeLists.txt:55 (ADD_SRC_SUBFOLDER)

CMake Error at cmake_modules/AddSrcSubfolder.cmake:18 (INCLUDE):
  include could not find load file:

    ../cmake_modules/debugger/CMakeLists.txt
Call Stack (most recent call first):
  src/CMakeLists.txt:58 (ADD_SRC_SUBFOLDER)


A deeper look is needed to find out why the relative path is wrong.

Results of a second look at it after dinner:

cmake_modules/AddSrcSubfolder.cmake  contains the ADD_SRC_SUBFOLDER and RELATIVE_PATH macros. These are used e.g. in src/zorbatypes/CMakeLists.txt where source sub-folders are added.

What I see here is that when the RELATIVE_PATH command in AddSrcSubfolder.cmake is evalulated, CMAKE_CURRENT_LIST_FILE contains the full path to cmake_modules/AddSrcSubfolder.cmake, a wrong relative path is calculated because of that, and in turn it fails to include the sub-folder CMakeLists.txt files. Hence the errors as above. If I patch it to evaluate CMAKE_PARENT_LIST_FILE instead, it works until cmake moves from the top-level "src" directory into the top-level "test" directory.

The cmake-2.6.3-2.fc10 test update is affected by this whereas cmake-2.6.2-3.fc10 in stable is not.

I think the memory misuse bug in CMake definitely needs fixing, can you please post the patch you used in comment #3?

As for the remaining issue, it looks like the behavior of CMAKE_CURRENT_LIST_FILE changed somehow from 2.6.2 to 2.6.3? What does CMAKE_CURRENT_LIST_FILE contain at the same place in 2.6.2?

I just tried CMake 2.6.3 by installing it from source on a Fedora 10 machine.
With the sources reported in bug 457160 I get the same cmake problem.
I also don't see the problem with cmake-2.6.2.fc10.   So there is something broke in CMake 2.6.3.

Created attachment 335252 [details]
experimental patch to fix C-string corruption in cmMakefile.cxx

Here's what I changed to simply fight the symptoms (= garbage or "invalid args error" as a result of freed string storage).

What I haven't done is to trace the AddDefinition method to find out where this C-string is stored in the end, e.g. whether and how it is copied into a std::string.

[...]

> it looks like the behavior of CMAKE_CURRENT_LIST_FILE changed
> somehow from 2.6.2 to 2.6.3? What does CMAKE_CURRENT_LIST_FILE
> contain at the same place in 2.6.2?

That's a matter of definition. The existing documentation is terse. I assume with 2.6.2 the global points to the CMakeLists.txt file in the current "src" subdir, e.g. ./src/zorbatypes/CMakeLists.txt, before entering the source subfolders in that directory. As I see directory stack operations in the code, the changed behaviour could be side-effects.

If this in Zorba's ADD_SRC_SUBFOLDER,

	FILE(RELATIVE_PATH REL_PATH ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_LIST_FILE})

is based on a wrong assumption (even if it used to work prior to 2.6.3), CMake cannot be blamed.

I noticed that cmake 2.6.4 is now available.   I tried it on the sources of
 bug 457160 which originally reported this bug and things worked fine.
So if the maintainer can update to cmake 2.6.4, this issue will be resolved.

I can confirm that this bug is fixed with cmake-2.6.4-1.rpm on Fedora 10.

cmake-2.6.4-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.