Bugzilla Bug 168077

I have sent this bug report to Trolltech, but have also been instructed by
William L. Thomson Jr. to file the bug here.  

The problem is more likely 'Firebird's Fault' than Qt's, although many libs
that use Firebird have a workaround, except Qt.  The problem is: Firebird
declares it's handle for all its objects as void* on 32-bit architectures.  On
64-bits it delcares it as int32, simply because it should ALWAYS be a 32bit
number, regardless of architecture; it was a very unfortunate choice of
declaration to begin with.  Qt fails to compile because it always expects a
void*.

This is the first patch I produce, and first bug report I post; if there is
anything wrong, please let me know and I'll fix it.

Patch follows:

diff -Naur a/src/sql/drivers/ibase/qsql_ibase.cpp
b/src/sql/drivers/ibase/qsql_ibase.cpp
--- a/src/sql/drivers/ibase/qsql_ibase.cpp      2006-11-27 15:26:28.000000000
-0200
+++ b/src/sql/drivers/ibase/qsql_ibase.cpp      2007-02-05 23:01:38.000000000
-0200
@@ -36,8 +36,6 @@

 #include <qdebug.h>

-#include <ibase.h>
-
 #include <stdlib.h>
 #include <limits.h>
 #include <math.h>
@@ -1168,7 +1166,11 @@

 QVariant QIBaseResult::handle() const
 {
+    #if defined(_LP64) || defined(__LP64__) || defined(__arch64__)
+    return QVariant(d->stmt);
+    #else
     return QVariant(qRegisterMetaType<isc_stmt_handle>("isc_stmt_handle"),
d->stmt);
+    #endif
 }

 /*********************************/
@@ -1179,11 +1181,11 @@
     d = new QIBaseDriverPrivate(this);
 }

-QIBaseDriver::QIBaseDriver(void *connection, QObject *parent)
+QIBaseDriver::QIBaseDriver(isc_db_handle connection, QObject *parent)
     : QSqlDriver(parent)
 {
     d = new QIBaseDriverPrivate(this);
-    d->ibase = (isc_db_handle)connection;
+    d->ibase = connection;
     setOpen(true);
     setOpenError(false);
 }
@@ -1448,5 +1450,9 @@

 QVariant QIBaseDriver::handle() const
 {
-    return QVariant(qRegisterMetaType<isc_db_handle>("isc_db_handle"),
d->ibase);
+    #if defined(_LP64) || defined(__LP64__) || defined(__arch64__)
+    return QVariant(d->ibase);
+    #else
+    return QVariant(qRegisterMetaType<isc_stmt_handle>("isc_stmt_handle"),
d->ibase);
+    #endif
 }
diff -Naur a/src/sql/drivers/ibase/qsql_ibase.h
b/src/sql/drivers/ibase/qsql_ibase.h
--- a/src/sql/drivers/ibase/qsql_ibase.h        2006-11-27 15:26:28.000000000
-0200
+++ b/src/sql/drivers/ibase/qsql_ibase.h        2007-02-05 22:58:24.000000000
-0200
@@ -24,6 +24,7 @@
 #ifndef QSQL_IBASE_H
 #define QSQL_IBASE_H

+#include <ibase.h>
 #include <QtSql/qsqlresult.h>
 #include <QtSql/qsqldriver.h>
 #include <QtSql/private/qsqlcachedresult_p.h>
@@ -62,7 +63,7 @@
     friend class QIBaseResultPrivate;
 public:
     explicit QIBaseDriver(QObject *parent = 0);
-    explicit QIBaseDriver(void *connection, QObject *parent = 0);
+    explicit QIBaseDriver(isc_db_handle connection, QObject *parent = 0);
     virtual ~QIBaseDriver();
     bool hasFeature(DriverFeature f) const;
     bool open(const QString & db,

Created an attachment (id=111018) [details]
The patch

Reassigning to qt to get a resolution. Don't really see how this fix should
fall on arch team, despite it effecting only one arch I believe. However my gut
says if it fails on amd64 likely would on any 64bit arch.

Actually looks like this might fall on me. Will have to see if the problem
still exists with Firebird 2.0. If not then is only a 1.5.x problem. If both do
then I will see about making patch and applying.

This is a 2.0 problem for sure, since I've only tested it in the 2.0.x series. 
I haven't tested it against 1.5 or 1.0, but it is probably there since it looks
like very old code.

Have you tested against 2.0.1? It was release after the bug was opened.
Firebird and it's code base has been around for decades. I thought they went
through and updated all code, but seems there is still a good deal of legacy
code in the code base.

Just tested against FB 2.0.1 and 2.1 Alpha-1 and Qt 4.2.3

The problem lies in a declaration in FB's header files, and it isn't really
going to be fixed by firebird.  The firebird should (and maybe it will from
what I've talked to them about this) change its declaration from void* to uint
on ALL platforms, this would then break the Qt plugin on all platforms. ;-)

Regardless, Qt should fix its code to cope with this problem.  FB handles are
always 32-bit numbers.  It just is declared as void* in 32-bit archs, and uint
in 64bit ones.

*** Bug 178747 has been marked as a duplicate of this bug. ***

Well the other is not really a 100% duplicate but has related issues. The other
bug marked a duplicate of this one seems to have issues with Firebird's header
file during tests but not during build? I just built qt-4.3.0_rc1 against
firebird-2.0.1.12855.0-r3 and all that was needed was to symlink
/opt/firebird/include/ibase.h in /usr/include. But I think that's just for the
test. I need to rule that out.

So the patch was not needed to build here? Should the build fail or what? I am
assuming it should. Per the other bug I believe it failed at the test part.

I don't think its a duplicate.  This bug assumes that Qt has found the ibase.h
file, the problem lies within the file; and it is specific to 64-bit arhes,
thanks to a "#if defined(_LP64) || defined(__LP64__) || defined(__arch64__)"

I've just looked at the sources for Firebird 1.5, and apparently they don't
even compile on AMD64. From the Firebird Support Forum: 

"The missing piece here (as far as I can tell from what you have told us) is
that nobody has done an AMD64 port of Fb 1.5.x that has been submitted to
Firebird QA."

(In reply to comment #9)
> I don't think its a duplicate.

No, but related. Both are about qt failing to build with Firebird use flag.

>  This bug assumes that Qt has found the ibase.h
> file, the problem lies within the file; and it is specific to 64-bit arhes,
> thanks to a "#if defined(_LP64) || defined(__LP64__) || defined(__arch64__)"

Yes, I need to test that out. But atm it seems the firebird use flag for qt is
use.masked, or masked in the profile.

> I've just looked at the sources for Firebird 1.5, and apparently they don't
> even compile on AMD64. From the Firebird Support Forum: 
> 
> "The missing piece here (as far as I can tell from what you have told us) is
> that nobody has done an AMD64 port of Fb 1.5.x that has been submitted to
> Firebird QA."

That's pure FUD. I went upstream with 1.5.3 when we de-stabilized that per
upstreams comments. Upstream tends to not like anyone building firebird from
source, and prefers that they run the binary releases.

That said we used to have patches for 1.5.3 on amd64. Now we are applying all
of Debians patch set at the moment (which I dislike some is ok, but not all). I
haven't tested less the patches but 1.5.4 should build on amd64. If it does
not, we have patches that allow for that.

I run ~amd64 no my laptop, and my production firebird server is also amd64. I
have 1.5.4 running on it without problems. The production firebird has run
1.5.3 for years on it. Not to mention clients amd64 production servers also
running 1.5.3 for quite some time. Despite upstreams comments and views.

Let me do some further testing with Firebird 2.0.1 and qt on amd64.

Well, uhm, sorry about the FUD.  It did seem a valid point since I saw no
compiler directives in the ibase.h file for 64bit arches, but if you got it
running, then it simply proves me wrong ;-)

Anyway, I'm looking at the sources for PHP, and adding these parameters to its
configure does the trick:

./configure {args} --with-interbase=/opt/firebird
--with-pdo-firebird=/opt/firebird 

for Qt it would be:

./configure {args} -I /opt/firebird/include/ -L /opt/firebird/lib/

Although, during my tests, Qt never seemed to have a problem finding the files.

(In reply to comment #11)
> Well, uhm, sorry about the FUD.  It did seem a valid point since I saw no
> compiler directives in the ibase.h file for 64bit arches, but if you got it
> running, then it simply proves me wrong ;-)

Negative, that was on ~x86 and x86. On ~amd64 I have confirmed the problem.

g++ -c -O2 -march=k8 -mtune=k8 -O2 -march=k8 -mtune=k8 -fvisibility=hidden
-fvisibility-inlines-hidden -D_REENTRANT -Wall -W -fPIC -DQT_NO_CAST_TO_ASCII
-DQT_NO_CAST_FROM_ASCII -DQT_NO_DEBUG -DQT_PLUGIN -DQT_SQL_LIB -DQT_CORE_LIB
-D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE -DQT_SHARED
-I../../../../mkspecs/linux-g++ -I. -I../../../../include/QtCore
-I../../../../include/QtCore -I../../../../include/QtSql
-I../../../../include/QtSql -I../../../../include -I.moc/release-shared
-I.uic/release-shared -o .obj/release-shared/qsql_ibase.o
../../../sql/drivers/ibase/qsql_ibase.cpp
../../../sql/drivers/ibase/qsql_ibase.cpp: In constructor
'QIBaseDriver::QIBaseDriver(void*, QObject*)':
../../../sql/drivers/ibase/qsql_ibase.cpp:1247: error: cast from 'void*' to
'isc_db_handle' loses precision
../../../sql/drivers/ibase/qsql_ibase.cpp: In member function 'virtual bool
QIBaseDriver::open(const QString&, const QString&, const QString&, const
QString&, int, const QString&)':
../../../sql/drivers/ibase/qsql_ibase.cpp:1312: warning: cannot pass objects of
non-POD type 'class QString' through '...'; call will abort at runtime
make[3]: *** [.obj/release-shared/qsql_ibase.o] Error 1
make[3]: Leaving directory
`/tmp/portage/x11-libs/qt-4.3.0_rc1/work/qt-x11-opensource-src-4.3.0rc1/src/plugins/sqldrivers/ibase'
make[2]: *** [sub-ibase-all] Error 2
make[2]: Leaving directory
`/tmp/portage/x11-libs/qt-4.3.0_rc1/work/qt-x11-opensource-src-4.3.0rc1/src/plugins/sqldrivers'
make[1]: *** [sub-sqldrivers-all] Error 2
make[1]: Leaving directory
`/tmp/portage/x11-libs/qt-4.3.0_rc1/work/qt-x11-opensource-src-4.3.0rc1/src/plugins'
make: *** [sub-plugins-all-ordered] Error 2

!!! ERROR: x11-libs/qt-4.3.0_rc1 failed.


> Anyway, I'm looking at the sources for PHP, and adding these parameters to its
> configure does the trick:
> 
> ./configure {args} --with-interbase=/opt/firebird
> --with-pdo-firebird=/opt/firebird 
> 
> for Qt it would be:
> 
> ./configure {args} -I /opt/firebird/include/ -L /opt/firebird/lib/
> 
> Although, during my tests, Qt never seemed to have a problem finding the files.

qt is for sure having a problem during it's test with ibase.h not being in
/usr/include. Funny thing is at build it finds the header files without a
problem. So is specific to the test only. Likely not using the same paths and
etc when that file is compiled and etc.

These qt problems are not 2.0.1 specific right? They are more Firebird on amd64
specific. Going to test 1.5.4 just to see, but expecting the same outcome.
Since 2.0.1 should be 100% backwards compatible.

Either way, we have to do something about the test failing. I can reproduce
that on all archs, and I don't think placing symlinks for the header files in
/usr/include is the way to go. Although I have no real objections to doing
that.

Then we just have to decide on the qt problem, do we modify Fb via the patch
you provided above. Or modify qt and submit a patch to them? We can always try
to push the above patch to Fb upstream as well.

The patch I've submitted originally was for Qt.  It was more a demonstration of
the bug and possibly how to resolve it.  Looking at the problem, there is no
patch I can think of for Firebird.  Firebird typedef's its handle type as
FB_API_HANDLE (which changes from void* in 32bit to uint in 64bit).

Qt *SHOULD* have used FB_API_HANDLE instead of assuming it would alwyas be
'void*'.

My vote is to patch Qt.  I've already sent a bug report to them about this
issue, no news until now.

Regarding copying stuff to /usr/include, I think it is a good idea, and then it
would make sense to copy the shared libs in /opt/firebird/lib to
/usr/lib{/firebird}?

(In reply to comment #13)
>
> My vote is to patch Qt.  I've already sent a bug report to them about this
> issue, no news until now.

I agree. Did you provide patches to upstream? If so please provide them here as
well. If upstream does not deal, or in between now and when they do. I can
likely get qt maintainers to apply patch when fb use flag is set on 64bit
arches.

> Regarding copying stuff to /usr/include, I think it is a good idea, and then it
> would make sense to copy the shared libs in /opt/firebird/lib to
> /usr/lib{/firebird}?

Long term ideally both will reside in those locations only. For now I am
symlinking the stuff in /opt/firebird/lib to /usr/lib. I can do the same with
the include stuff.

Debian applies patches to let fb be split up like that for proper FHS
compliance and etc. I will likely have to go the same route. There is a issue
with the log file and lock files possibly. So all in time for sure. For now
just trying to address what is needed with band aids.

(In reply to comment #14)
> I agree. Did you provide patches to upstream? If so please provide them here as
> well. If upstream does not deal, or in between now and when they do. I can
> likely get qt maintainers to apply patch when fb use flag is set on 64bit
> arches.

Yes I did, and it is the same patch which I've attached to this bug report
originally.  I've hard nothing from them since.

If needed, I can recreate the patch against the newer sources ( Qt 4.2.3 ),
just to be sure, or if the patch isn't in the correct format.  I've never
created a patch before.  ;-)

(In reply to comment #15)
>
> If needed, I can recreate the patch against the newer sources ( Qt 4.2.3 ),
> just to be sure,

Couldn't hurt. Than I for sure can take to our qt peeps and/or cc them on this
bug to get them involved.

 or if the patch isn't in the correct format.  I've never
> created a patch before.  ;-)

Your inline patch is good. What ever the attachment is to this bug seems binary
and that's not ideal :) Ideally the patch should be attached to bug, but format
is find, I use -Naur myself. So if they are fine we can work with them inline
for now :) Or if you want to get practice attaching it as a file to this bug.

Created an attachment (id=119731) [details]
isc_db_handle fix

This changes the wrong "void*" types to "isc_db_handle".  This fixes the build
on all platforms.

The driver made an erroneous assumption that the handle type was a 'void*',
which would break on 64bit platforms.

Created an attachment (id=119732) [details]
QIBaserDriver::Handle() fix

Back ported a missing '&' for the driver for versions 4.1.x from 4.2.3

Here are the new patches.  Apparently, the driver has change since I first
created the patch, and this affects directly the 64bit build.  So, the
isc_db_handle.patch is the only file needed for the current version (4.2.3) of
Qt.

When I applied that patch on older sources (4.1.5), the build broke again, this
time because of the line:

return QVariant(qRegisterMetaType<isc_db_handle>("isc_db_handle"), d->ibase);

which on newer version is:

return QVariant(qRegisterMetaType<isc_db_handle>("isc_db_handle"), &d->ibase);

that is what the second patch fixes.

I tried to look up the bug report I filed with TrollTech on this issue, but it
seems as though their site is broken for now.


To sum up - for those of you just joining us - the QIBaseDriver plugin fails to
build on the AMD64 platform because it wrongfully assumes that the handles for
objects (like statement, database connection, etc) is a 'void*'.  This is only
partially true for 32bit arches; because the handle should *ALWAYS* be a
unsinged int.  The fix is to simply replace 'void*' for isc_db_handle which is
defined in the ibase.h file, and will always be the correct type on all
platforms.

This is not the end of the story though.  After building the plugins, it seemed
that the 4.1.5 code is _still_ broken (everything in 4.2.3 looked perfect). 
When using the sqlbrowser example supplied with qt, all integer fields in my
database were being corrupted.  It looks like a singed/unsigned issue.  For
example, instead of a value of '1657', it showed '47240345290361'.  I'll try to
trace the issue later, to know exactly where it is; and try to build and
reproduce the problem on my Slack-32bit machine.  But should I even bother? 
Since on the newer version, it works fine?

Can you sum up which patch to use with on which qt4 version.  Note that 4.3 is
now in portage.

Patches needed:

 4.1.x - isc_db_handle fix  and  QIBaserDriver::Handle() fix
 4.2.x - isc_db_handle fix
 4.3.0 - isc_db_handle fix

I've just tested the ebuild for firebird 2.0.1.12855.0-r3.  I found two
problems with it that I think are realted to these issues:

- It failed to load, because it couldn't find 'libfbembed.so.2'.  That file was
supposed to be in /opt/firebird/lib but since my machine is an AMD64, that file
was placed in /opt/firebird/lib64.  A symlnk lib -> lib64 fixed that.

- It said that the 'ISO8859_1' charset wasn't installed.  All my db's use that
so I could connect to the database, but not fetch any data.  No solution found.
 UTF worked.

(In reply to comment #22)
> That file was
> supposed to be in /opt/firebird/lib but since my machine is an AMD64, that file
> was placed in /opt/firebird/lib64.  A symlnk lib -> lib64 fixed that.

The ebuild should be creating that symlink. Was it not present for you after
merge? If the lib dir remains, symlink creation will fail.

FYI, amd64 stable does not create lib64, quite a pita. Diff between ~arch and
arch there.

> - It said that the 'ISO8859_1' charset wasn't installed.  All my db's use that
> so I could connect to the database, but not fetch any data.  No solution found.
>  UTF worked.

Yeah I am plagued by that same problem. It's quite annoying and not sure how to
fix atm. I recall they moved most all stuff to UTF, now that they have proper
unicode support. But still ISO8859_1 should work. Not sure what's going wrong
at build, or is missing from runtime.

(In reply to comment #23)
> The ebuild should be creating that symlink. Was it not present for you after
> merge? If the lib dir remains, symlink creation will fail.
> 
> FYI, amd64 stable does not create lib64, quite a pita. Diff between ~arch and
> arch there.
Yes it did, my bad.  After a clen re-install it created the symlink.


> Yeah I am plagued by that same problem. It's quite annoying and not sure how to
> fix atm. I recall they moved most all stuff to UTF, now that they have proper
> unicode support. But still ISO8859_1 should work. Not sure what's going wrong
> at build, or is missing from runtime.

I found the problem.  It is during the install phase.  Two files are missing:

intl/fbintl
intl/fbintl.conf

The fbintl file is generated as libfbintl.so, but it needs to be renamed and
given exec perms.  Doing that manually worked for me.

Created an attachment (id=121765) [details]
firebird-2.0.1.12855.0-r4.ebuild

Proposed ebuild to fix missing character sets.

Thanks for finding the character set problem. FYI, a patch is more ideal than
attaching a full ebuild :) But no worries, just committed the changes in
2.0.x-r4

Just realized I never committed that fix for character sets.

Also I hope progress is made on this bug. And the other. Firebird < 2.0.1 has a
security issue and needs to be stabilized ASAP. I will add this bug as a
blocker to that one. So we make sure to address the qt issues as Firebird goes
stable. So users don't bitch about a broken stable trees. As it would be fully
in their right to do so :)

I've sent a follow-up e-mail to TrollTech and just received an  telling me that
the bug is scheduled for 4.4.0:  

http://trolltech.com/developer/task-tracker/index_html?id=149761&method=entry

(Note that they just changed it to high priority and scheduled it after I
inquired its status, i.e. yesterday.  Guess I should have done this earlier,
might had made it in 4.3 ;-) )

(In reply to comment #27)
>So we make sure to address the qt issues as Firebird goes
> stable. So users don't bitch about a broken stable trees. As it would be fully
> in their right to do so :)

A security issue is no valid argument to break the stable tree.

There is nothing stopping us from applying said patch to qt when the firebird
use flag is set on the effected archs. We don't have to wait for upstream there
right? That would prevent any breakage, but would need to stabilize that
version of qt along with Firebird. Which did recently get stabilized on x86.

Qt 4.3.1 now builds fine with Firebird on AMD64.  As far as I can tell, this
bug was fixed by TrollTech; apparently they applied my patch to their codebase. 

So long, and thanks for all the fish!

So is this resolved now and can we close the bug?

Yes.  The firebird plugin now builds on AMD64 perfectly.  As far as I'm
concerned, this bug can be closed.

Ok closing bug, please re-open if problem remains. Thanks all for helping out
with this one, via patches and etc. Much appreciated :)