Archived

This forum has been archived. Please start a new discussion on GitHub.

slice2cpp generating odd code with 3.4 and 3.5b

The minimal testcase below is also here: testice.tar. Note that it's only to demonstrate the problem, it's not directly useful.

With the following slice definition:
module test {
    class Foo
    {
      long   no;
    };

    class Bar
    {
      string s;
      Foo f;
    };
};
#include <IceUtil/Handle.h>
#include "test.h"

int main()
{
  IceUtil::Handle<test::Foo> fh(new test::Foo);
  IceUtil::Handle<test::Bar> bh(new test::Bar);

  return 0;
}

I get different inheritance hierarchies in the generated code depending upon the Ice version. With Ice 3.3:
class Foo : virtual public ::Ice::Object
class Bar : virtual public ::Ice::Object

With Ice 3.4 and 3.5beta:
class Foo : virtual public ::Ice::Object
class Bar : virtual public ::Ice::Object, private IceInternal::GCShared

With the 3.3 version, the inheritance is linear:
IceUtil::Shared
IceInternal::GCShared
Ice::Object
Foo|Bar

With the 3.4/3.5 version, it has multiple bases for Bar:
IceUtil::Shared
Ice::Object        IceInternal::GCShared
               Bar

This causes problems:

In this example, IceUtil::Handle<Foo> works. It still has a single base. IceUtil::Handle<Bar> is broken; it has both IceUtil::Handle and IceInternal::GCShared. Since both of these provide __incRef() and related methods, it's not possible to compile:
$ make CXX=g++-4.4
g++-4.4 -I. -o test test.cpp main.cpp -lIce -lIceUtil
In file included from main.cpp:1:
/usr/include/Ice/GCShared.h: In constructor ‘IceUtil::Handle<T>::Handle(T*) [with T = test::Bar]’:
main.cpp:11:   instantiated from here
/usr/include/Ice/GCShared.h:33: error: ‘virtual void IceInternal::GCShared::__incRef()’ is inaccessible
/usr/include/IceUtil/Handle.h:168: error: within this context
/usr/include/IceUtil/Handle.h:168: error: ‘IceInternal::GCShared’ is not an accessible base of ‘test::Bar’
/usr/include/Ice/GCShared.h: In destructor ‘IceUtil::Handle<T>::~Handle() [with T = test::Bar]’:
main.cpp:11:   instantiated from here
/usr/include/Ice/GCShared.h:34: error: ‘virtual void IceInternal::GCShared::__decRef()’ is inaccessible
/usr/include/IceUtil/Handle.h:197: error: within this context
/usr/include/IceUtil/Handle.h:197: error: ‘IceInternal::GCShared’ is not an accessible base of ‘test::Bar’
make: *** [test] Error 1

This appears to occur for all generated classes which have objects derived from Ice::Object as members, but not for simpler classes which do not.

Given that it was possible to create an IceUtil::Handle<T> for any object derived from Ice::Object in Ice 3.3, and this appears to not be the case in Ice 3.4/3.5beta, is there an alternative I should be using instead, or is this a bug in either my code or in the generated code?
Any recommendations would be greatly appreciated.


Many thanks,
Roger

Comments

  • bernard
    bernard Jupiter, FL
    Hi Roger,

    Thank you for this bug report. We noticed this issue recently and unfortunately did not address it for Ice 3.5b.

    For now, the work-around is to use the generated Ptr typedef, like MyClassPtr; if you are writing your own derived class, you can use IceInternal::Handle instead of IceUtil::Handle as a temporary work-around.

    Best regards,
    Bernard
  • Hi Bernard,

    Many thanks for the suggestion. Using IceInternal::Handle typedefs has got us most of the way to making our code work with Ice 3.4/3.5b, though we were required to also add a large number of IceInternal::upCast() definitions to our code as well, and even now we are still tracking down remaining runtime issues. The ongoing maintenance burden of this workaround will be significant. Is this something which will be properly addressed in Ice 3.5?


    Kind regards,
    Roger
  • benoit
    benoit Rennes, France
    Yes, this is already fixed on our mainline. You could also fix the slice2cpp translator to instead inherit publicly from GCShared rather than privately. The fix is trivial: just change "private IceInternal::GCShared" to "public IceInternal::GCShared" in cpp/src/slice2cpp/Gen.cpp from your Ice source distribution and re-compile the slice2cpp translator.

    Cheers,
    Benoit.
  • Dear Benoit,

    I can confirm that applying this one line change to Gen.cpp does correct the problem, many thanks. Will this fix be backported to a 3.4.x point release as well as being fixed in 3.5?

    Is the source code repository publicly available? It would make it easier to track the development and catch issues such as this earlier, as well as to better enable us to send you patches to correct things as we find them.


    Regards,
    Roger
  • bernard
    bernard Jupiter, FL
    Hi Roger,

    It took a long time before we became aware of this issue, so it's apparently not a very common situation.

    The next Ice release will be Ice 3.5.0; we're not planning to release a Ice 3.4.3 with bug-fixes. If you have a support contract and need this fix back-ported to Ice 3.4, please submit a support request.

    The Ice source code for all Ice versions is available from our download page. We do not maintain a public source code repository.

    Best regards,
    Bernard
  • Dear Bernard,

    Backporting the fix is absolutely not a problem. I've done this for both 3.4 and 3.5b and that works just fine. I'll explain the background to the issue here, because it covers quite a bit more than just the immediate problem in this thread.

    For people distributing precompiled binaries, backporting a fix, rebuilding Ice, and then rebuilding your code against the new Ice build is fairly trivial. You can then distribute these binaries and the problem is solved.

    For people distributing source, we can certainly do the same. However, we only fix the problem for ourselves. Every single consumer of our source, be it other developers, users, Linux distributors, commercial customers etc. is required to repeat the process. Fixing it locally for myself doesn't solve the wider issue. It's still broken for everyone else except me. For this reason, having a bugfix release is essential, since otherwise the Ice releases are broken by default, and this has unfortunate implications for being able to continue using Ice both for free software/open source and commercial purposes. We can't use it if it doesn't work.

    I can go a step further and push these patches onto the Linux distribution maintainers. It'll take just a couple of minutes to e.g open a bug report and patch for the Debian maintainer to apply. But this still doesn't really solve the problem. It just makes things slightly less painful for a small subset of users. It's still broken for everyone else not using Debian. And more importantly, this comes at the significant expense of source and binary compatibility with the official Ice releases and with other distributions, so isn't at all tenable due to this divergence for all but the most trivial of changes.

    Just to highlight some of the problems we are experiencing:
    • Ice <= 3.4 is unusable with current compilers due to generating invalid C++. Fixing this using your patches breaks binary compatibility. This makes Ice broken by default on all contemporary Linux systems, and any systems using a current GNU or LLVM toolchain. This adds an extra dimension of pain in using Ice at all on current systems, where everything else is using current tools. I know this will be much better in 3.5, but it is a major problem which can't be safely addressed other than by an official point release. E.g. any Linux distributor who makes this change breaks the ABI. This almost got Ice dropped from Debian by the release managers.
    • 3.4 is unusable for users using IceUtil::Handle with their own objects. While this works in 3.3 and (eventually) in 3.5, 3.4 is utterly broken in this situation. And without an official release, will remain so due to the reasons given above.

    Examples of why locally patching does not work, and causes large amounts of pain, as experienced by Debian:

    #672066 - zeroc-ice34: slice2cpp generates invalid C++ code - Debian Bug report logs
    zeroc-ice34: slice2cpp generates invalid C++ code
    "Please revert the patch applied to fix this bug in -8.
    ...
    'Therefore a large portion of this patch is moving upCast overloads out of IceInternal into the Ice namespace.' Which leads to bugs like #675955 in every package using this. If no other actually sane solution can be found, then you'll need to force this package to build with gcc-4.6 instead. If upstream is really going to apply this patch, they'll need to bump SONAME and do a proper transition for it."
    "* Revert the patch 'fixing' the FTBFS with gcc-4.7 that breaks ABI, and add force_gcc_4.6.patch. We need to do this all in the upstream makefile because it has a silly heck for the intel icpc compiler that we need to patch around or the build will fail with anything but c++ as the compiler..."
    "Post-wheezy, it is likely that gcc-4.6 will no longer be included and this bug will again require attention for jessie. For this reason, I'm marking the bug 'wheezy-ignore' rather than downgrading it. Hopefully once wheezy is out, an upstream fix will be available with a proper SONAME bump for this issue."

    #675955 - mumble-server: Error launching mumble-server 1.2.3-348 - Debian Bug report logs
    "You'll probably need to go back to that version for now. It turns out the cause of this is the 'fix' for #672066 in zeroc-ice which breaks ABI. That will need to be reverted, and all affected packages will need to be rebuilt.
    "Until that happens, things are probably going to be a bit chaotic both in sid and wheezy for things using zeroc-ice."
    "* Build-Depend on the unfycked version of zeroc-ice, so the ABI broken one doesn't accidentally get used by an out of date buildd. Closes: #675955. Force building with gcc-4.6, since that's required for zeroc-ice deps now, until they get their act together and write some actually legal code."

    Apologies for the long post. I just wanted to point out that backporting patches is not an option in all cases, and this often causes additional problems, particularly relating to ABI/API compatibility. At present, we are finding it increasingly difficult to continue to use Ice given that we need a working Ice, but there are no officially released versions which work with up-to-date toolchains and systems.

    Regards,
    Roger
  • bernard
    bernard Jupiter, FL
    Hi Roger,

    Thank you for the feedback.

    Please keep in mind that we build, test and support Ice only with a limited set of OS and compilers. See ZeroC - Supported Platforms for Ice 3.4.2.

    While we try hard to write portable code, there are occasionally portability problems with newer platforms or compilers. The two posts you referenced are about build failures with gcc 4.7 on Debian "wheezy" - the codename for the next major release of Debian. Yes, it would be nice if older versions of Ice compiled with gcc 4.7 on this cutting-edge Debian. The situation is however not as dire as your post suggests: most Linux distributions don't include such recent versions of gcc. And when they do, Ice 3.5 will be available.
    Ice <= 3.4 is unusable with current compilers due to generating invalid C++. Fixing this using your patches breaks binary compatibility.

    If you look more closely at this gcc 4.7 patch for Ice 3.4.2, it is a patch posted on our forums by an Ice user like yourself. I would characterize it as "experimental", in particular it's not an official ZeroC patch!
    3.4 is unusable for users using IceUtil::Handle with their own objects. While this works in 3.3 and (eventually) in 3.5, 3.4 is utterly broken in this situation. And without an official release, will remain so due to the reasons given above.

    This is a bug, yes. Fortunately it does not appear to affect many users or projects: you were the first to report this issue, 3 weeks ago, several years after the initial release of Ice 3.4.

    We are keenly aware of binary compatibility issues, and a main reason for the Ice 3.5 beta release is to give you the ability to report problems before the API is frozen, while can still fix bugs without binary-compatibility constraints.

    Thank you for testing Ice 3.5b! Ice 3.5.0 will soon incorporate a fix for this Handle bug.

    However, I don't see what you would like us to do for Ice 3.4. Research a binary-compatible bug-fix, and, if successful, publish an official patch? This may not be doable.

    Best regards,
    Bernard