Segmentation fault building on Ubuntu 12.04 using Dune ppa about opm-upscaling HOT 45 CLOSED

@alfbr
Do you have a Minimum Working (or at least compiling :-)) Example?

from opm-upscaling.

Also, this should be checked on an Ubuntu Quantal machine. The PPA packages are (supposed to be) copies of the packages made by the Debian Science team with basically only dependency information changed to suite Precise. (Meaning: there is now a chance that it segfaults on any newer Ubuntu/Debian machine with system packages).

from opm-upscaling.

It seems to be easily reproducible, just install dune from the ppa and compile opm with dunecontrol. Your resulting binaries will segfault when you try to execute them. Not sure when I will have time to check on 12.10, but I will of course report back if I do.

from opm-upscaling.

The samples crash in the static initialization code. At first glance, this appears to happen in a field initialized in an ISTL header, which is included in several translation units (meaning the variable will get initialized twice)

Since ISTL has no library, I currently cannot say why this should work when linking to a custom build of DUNE and not the PPA; it may be that the initialization order is different for some reason.

Also, if I change linking of opmcore to be static instead of shared (the other modules are only available for static linking), then the error goes away(!).

from opm-upscaling.

So... should we default to building a static library only?

This is a question for the whole chain of libraries.

from opm-upscaling.

@atgeirr: Not until we establish that static-only builds is a solution rather than a work-around.

from opm-upscaling.

Hey,

we always refrained from building a library in dune-istl. Maybe we
should change that.

In the meanwhile it would be most appropriate to build a library of
the headers in question within opm and use that one.

Markus

On Sun, Oct 21, 2012 at 11:03:09PM -0700, Atgeirr Flø Rasmussen wrote:

So... should we default to building a static library only?

---

Reply to this email directly or view it on GitHub:
#9 (comment)

Dr. Markus Blatt - HPC-Simulation-Software & Services http://www.dr-blatt.de
Hans-Bunte-Str. 8-10, 69123 Heidelberg, Germany, USt-Id: DE279960836

from opm-upscaling.

Linking the examples in opm-upscaling statically is a work-around which is (in my opinion) just as good solution as building DUNE from scratch. However, I clearly consider it a work-around until we figure out what is the underlaying problem.

from opm-upscaling.

@blattms
Isn't maybe the issue here that dune-istl puts initialization code in the headers, which is going to be included in the opm-xxx translation units anyway (and thus possibly/probably twice)?

from opm-upscaling.

On second thought: Might this be the bug http://www.dune-project.org/flyspray/index.php?do=details&task_id=1123 ?

from opm-upscaling.

The crash appears only in opm-upscaling and not in opm-core, right?

A double inclusion can only appear if two objects of a or two different librararies include the same istl header. In this case the static variable will be initialized and disposed twice, which might lead to a segfault.

Indeed there are opm/core/linalg/LinearSolverIstl.[hc}pp and opm-porsol/dune/porsol/common/LinearSolverISTL.[hc]pp which end up in libopmcore and libopmporsol, respectively. Whenever a program gets linked to both libraries there will be problems.

The reason why this does not happen for self compiled dune-modules is probably that opm-core does not find istl and therefore opm/core/linalg/LinearSolverIstl.[hc}pp is not in the library.

But I do not understand why static linking fixes the problem.

Is there a special reason why there are two ISTL interfaces in OPM? This always seemed disturbing to me,

from opm-upscaling.

The two ISTL interfaces are due to historical reasons. I think the one in opm-porsol should be removed, but I am a little reluctant to do so if it is true that opm-core does not find istl in some cases that it should. I'll try this out a little.

from opm-upscaling.

@alfbr
Alf, could you check if opm-core picks up ISTL from the self-compiled dune-modules; grep HAVE_DUNE_ISTL opm-core/config.h, in the build where the opm-upscaling examples do not crash?

from opm-upscaling.

@atgeirr: The ISTL detection in OPM-Core (i.e., the AX_DUNE_ISTL macro implemented in m4/ax_dune_istl.m4) assumes that ISTL is installed in a standard directory on the system (i.e., using make install). It won't find ISTL in a build tree.

from opm-upscaling.

@bska
I thought that dunecontrol was supposed to rig PKG_CONFIG_PATH so that it would pick up dependencies from the other modules, but maybe that was wishful thinking... :-)

from opm-upscaling.

On Mon, Oct 22, 2012 at 03:25:39AM -0700, Atgeirr Flø Rasmussen wrote:

The two ISTL interfaces are due to historical reasons. I think the
one in opm-porsol should be removed.

Mmh. Considering the current setup with opm-porsol being a dune module,
and thus have all the functionality (e.g. available 3rd party
software, compiler options, etc.) from dune-istl for sure, I am not
sure whether this is the best choice. To the very least it means
adapting the checks and defines whenever they change in dune-common
and dune-istl. Otherwise the settings of opm-core and the other opm
modules could differ to some extend with possible side effects.

from opm-upscaling.

@blattms
Isn't rather the situation that opm-porsol depends on dune-istl, and opm-core build with dune-istl support? Probably the opm-porsol .m4 scripts should be extended so that they check whether it is possible to link with LinearSolverIstl, and bail out if it isn't. Then LinearSolverIstl in opm-porsol can be retired.

from opm-upscaling.

@rolk: That (OPM-Porsol depending on ISTL and built-in support for ISTL in OPM-Core) seems, to me, to be the most straightforward description of the dependency graph. It will probably entail making the ISTL checks in OPM-Core more robust and imaginative, too.

from opm-upscaling.

OK, say we are on the trail of a solution here; adding checks in opm-porsol, phasing out LinearSolverISTL (capital letters, in opm-porsol) for LinearSolverIstl (camel letters, in opm-core). Then next question: If we have two translation units both using LinearSolverIstl, say on that solves pressure equation using some method in opm-core and one that contain some custom algorithm in a user program. The linker could probably merge them if one links statically (which is why the problem was circumvented by doing that), but if the units are in different objects (one in libopmcore.so and one in a.out) then what?

from opm-upscaling.

LinearSolverIstl is implemented in a .cpp file, so its code ends up only in the library. Maybe I did not understand the question?

from opm-upscaling.

@atgeirr
Yes, there was flawed reasoning; there will only be one LinearSolverIstl. The question is really what if someone links with dune-istl themselves, in addition to opm-core. (Maybe that's far-fetched, I don't know; what about Dumux?)

from opm-upscaling.

On Mon, Oct 22, 2012 at 04:23:13AM -0700, Roland Kaufmann wrote:

@atgeirr
Yes, there was flawed reasoning; there will only be one
LinearSolverIstl. The question is really what if someone links with
dune-istl themselves

dune-istl has no library. (Which probably is the root cause of the
problem.)

To keep other people from having to solve the same problem over and
over again, dune-istl should probably put the static initialization
code that cause the problem into a cc-file and create a library
itself. But that should not pose a problem as then the code is in the
istl library and not any more in the ones of OPM.

BTW: Has anybody actually investigated what code in dune-istl causes
the segfault?

from opm-upscaling.

@blattms
The segfault I got was from initializing AggregatesMap::UNAGGREGATED, at dune/istl/paamg/aggregates.hh (which is strange because it is a numeric field; why not just embed the constant?)

from opm-upscaling.

@blattms
Just a tangential note for those who aren't intimately familiar with the ISTL code: What ISTL files contain static initialisation code?

from opm-upscaling.

I tried to replicate the problem on my system by creating a dune module containing two libraries that have both one object with a method that calls the AMG of ISTL. Both are used in a binary and running does not segfault at all.

Are we sure that this really is the problem? Why should initializing a static cont std::size_t segfault, anyway?

The only differences to OPM are, that

• both objects are compiled with different config.h files
• opm-core is compiled with CXX="g++" while the rest of OPM uses CXX="g++ -std=c++0x"

Then I mimicked your OPM approach:
Created an a non-DUNE module that uses the AX_DUNE_ISTL and AX_DUNE_COMMON checks from OPM-core and consequently compiles one library without the -std=cxx+0x option and with a different config.h.
Now I get a segmentation fault, too.

Unfortunately, one cannot simply use CXX="g++ -std=c++0x" as one would have to reimplement all c++0x from DUNE in opm-core.

from opm-upscaling.

If change the code such that ist only calls the solver in the dune-module and compile the other module with CXX="g++ -std=c++0x" CXXFLAGS="-DHAVE_NULLPTR=1", to make it use the same compiler options, there is no segfault.

But if I also call the solver using the library of the non-DUNE module, this call still segfaults.

I consider this still a hack, but maybe someone can try with an opm-core compiled with the above flags.

from opm-upscaling.

@blattms
Just out of curiosity: What version of GCC are you using?

from opm-upscaling.

On Mon, Oct 22, 2012 at 09:45:11AM -0700, Bård Skaflestad wrote:

@blattms
Just out of curiosity: What version of GCC are you using?

4.7

Thanks for asking. If you are using 4.1, like on red hat 5, you should
not define HAVE_NULLPTR.

from opm-upscaling.

On Mon, Oct 22, 2012 at 03:38:34AM -0700, Roland Kaufmann wrote:

@bska
I thought that dunecontrol was supposed to rig PKG_CONFIG_PATH so
that it would pick up dependencies from the other modules

Actually it does. But if opm-core does not use pkg-config that is not
of much use.

from opm-upscaling.

I'm actually on 4.4 on Ubuntu 10.04 LTS at the moment. I haven't tried to replicate the issues mentioned earlier, but I thought I'd just briefly mention the following issue that came up in the GCC development list this summer:

http://gcc.gnu.org/ml/gcc/2012-06/msg00201.html

I'm not (nearly) knowledgeable enough to determine whether or not this information influences the current issue in light of CXX=g++ versus CXX="g++ -std=c++0x", but I thought I'd mention it anyway. I believe (but haven't checked) that GCC 4.7.2 was produced to, among other things, revert the ABI breakage that was introduced by the additional member of std::list.

from opm-upscaling.

Now that seems like a can of worms. More on ABI icompatibilities can be found at http://gcc.gnu.org/wiki/Cxx11AbiCompatibility

from opm-upscaling.

At the very least it warrants careful review of all libraries that are used together. Given that Dune's build system, at least the Autotools-based version, automatically appends -std=c++0x (or -std=c++11) to the CXX definition if the compiler supports that option, it goes without saying that OPM-Core should be built using the same setting.

from opm-upscaling.

@bska
So if opm-core is built with dunecontrol, it gets 'CXX="-std=c++0x"', but otherwise not?

from opm-upscaling.

@rolk
Well, almost. Dunecontrol (or, rather, the GXX0X macro defined in dune-common/m4/cxx0x_compiler.m4) automatically appends -std=c++0x to the CXX variable if the actual compiler supports that option. OPM-Core's build system does not inspect or modify CXX. In other words, OPM-Core is built using the CXX (and CXXFLAGS) defined by the builder. You might want to pass -std=c++0x (or some equivalent option) in CXXFLAGS when building OPM-Core.

from opm-upscaling.

@bska
OPM/opm-core#79 prevents us currently from building with those options (but why doesn't this fail with dunecontrol?)

from opm-upscaling.

@rolk
I'll see about reproducing OPM/opm-core#79 on CentOS 5 using a custom-built GCC 4.7. I'll report back when I have some further information (or questions).

from opm-upscaling.

When I now compiled with latest commit of opm-core (29ccdb), and with default options provided by OPM/opm-core#82, an executable which segfaulted before (examples/upscale_perm) no longer does, so I think Marcus' theory that the compiler gets the static initialization confused because different flags are used, is correct. Note that I am compiling with DUNE 2.2 installed from the PPA.

from opm-upscaling.

Assigned this to Alf: can you check if the issue is still relevant for you after the recent changes, and close it appropriate?

from opm-upscaling.

Sure, but it can take some time before I get to do it.

from opm-upscaling.

Is there any progress on this issue?

from opm-upscaling.

Sorry, no time to test.

from opm-upscaling.

@alfbr,

No problem. I just wanted to know the current situation. There is no rush.

from opm-upscaling.

While this issue raises some important questions about ensuring that all libraries used together are compiled with identical options etc., I am inclined to close it as no longer relevant for the upcoming release. Have anyone tested the current release branch together with Dune packages from PPA? I would be interested in hearing how that worked out.

from opm-upscaling.

Have anyone tested the current release branch together with Dune packages from PPA

This was my standard environment until a couple of pull-requests ago when I had to start testing the 'build-without-installed-DUNE'-scenario again; I haven't had a problem with that in months.

from opm-upscaling.

I will close this, then.

from opm-upscaling.