Opened 4 years ago

Closed 20 months ago

Last modified 19 months ago

#12301 closed defect (fixed)

Upgrade included libtiff to 4.0 to fix 64 bit build issues

Reported by: dsmtoday Owned by:
Priority: normal Milestone: 3.0.0
Component: 3rdparty Version: 2.9.1
Keywords: tiff 64bit Cc:
Blocked By: Blocking:
Patch: no

Description

When compiling recent versions of wxWidgets in 64bit mode under Visual Studio, a lot of warnings about size_t to int or unsigned pop up. Almost all of these are harmless and can easily be ignored. However, there is one warning about a size_t pointer to unsigned pointer that comes up, and it is dangerous.

There is an easy work around for this -- use a temporary unsigned variable and pass that to the function that is expecting a an unsigned pointer.

Here's the patch (and attached).

--- wxWidgets-2.9.1/src/tiff/libtiff/tif_dirinfo.c    2010-07-14 23:27:45.269903700 -0700
+++ wxWidgets\src\tiff\libtiff\tif_dirinfo.c    2010-07-31 12:12:39.043180700 -0700
@@ -737,6 +737,7 @@
 _TIFFFindFieldInfoByName(TIFF* tif, const char *field_name, TIFFDataType dt)
 {
     int i, n;
+    unsigned nfields;
 
     if (tif->tif_foundfield
         && streq(tif->tif_foundfield->field_name, field_name)
@@ -751,11 +752,13 @@
             key.field_name = (char *)field_name;
             key.field_type = dt;
 
+            nfields = tif->tif_nfields;
             ret = (const TIFFFieldInfo **) lfind(&pkey,
                          tif->tif_fieldinfo, 
-                         &tif->tif_nfields,
+                         &nfields,
                          sizeof(TIFFFieldInfo *),
                          tagNameCompare);
+            tif->tif_nfields = nfields;
         return (ret) ? (*ret) : NULL;
         } else
         for (i = 0, n = tif->tif_nfields; i < n; i++) {

Attachments (1)

out.txt download (874 bytes) - added by dsmtoday 4 years ago.
patch

Download all attachments as: .zip

Change History (12)

Changed 4 years ago by dsmtoday

patch

comment:1 Changed 4 years ago by neis

Note that tiff library is a third-party library, so we try to avoid any modifications. If you could check whether this is fixed in the current libtiff release and either report here that it is already fixed or report the bug to libtiff people if it is not yet fixed, this would be very helpful.

comment:2 follow-up: Changed 4 years ago by dsmtoday

  • Resolution set to wontfix
  • Status changed from new to closed

Okay, I checked the latest version of the libtiff library, and it isn't fixed there, either. The problem seems to come down to Microsoft defining _lfind as to take a pointer to an unsigned instead of taking a pointer to a size_t, which other libraries on other OSes seem to do.

But now that I think about it, everything will probably all work out anyway since an Intel processor is little endian. Thus, if you have a 64bit unsigned integer in memory that is always likely to be less than 32bits in value and thus have a zero in the top 32bits, and you pass a pointer to that 64bit integer to a function that expects a 32bit integer, and that function writes a new 32bit value into the bottom part of your 64bit integer, it all works out anyway.

So, while this is very sloppy code and would fail on a big endian 64bit machine with an _lfind library function that takes an unsigned pointer instead of a size_t pointer, it just isn't a problem we need to worry about here, I guess.

comment:3 in reply to: ↑ 2 Changed 4 years ago by neis

  • Milestone changed from 2.9.2 to 3.0
  • Resolution wontfix deleted
  • Status changed from closed to reopened

Replying to dsmtoday:

Okay, I checked the latest version of the libtiff library, and it isn't fixed there, either. The problem seems to come down to Microsoft defining _lfind as to take a pointer to an unsigned instead of taking a pointer to a size_t, which other libraries on other OSes seem to do.

Note however that it appears to be fixed in the tiff-4.0.0beta6 package (by replacing lfind by td_lfind), so once tiff-4.0 is released the fix should find its way into wx sooner or later...
I'll reopen it, so we won't forget there's a reason to update the included tiff library.

comment:4 Changed 4 years ago by neis

  • Status changed from reopened to confirmed

BTW, there seem to be several problems with 64 bit builds on windows according to libtiff's bugzilla, so upgrading to a version that has those fixed (once such a version is available) sure seems to be a good idea.

comment:5 Changed 4 years ago by vadz

Would be nice if someone could test whether wx TIFF support works correctly with libtiff 4.

comment:6 Changed 21 months ago by ghostvoodooman

any progress regarding this ticket?

comment:7 Changed 20 months ago by vadz

  • Keywords tiff 64bit added
  • Summary changed from 64bit pointer issue in tiff library to Upgrade included libtiff to 4.0 to fix 64 bit build issues

No, we still need someone to take time to upgrade libtiff to v4.

comment:8 Changed 20 months ago by VZ

(In [73112]) Update vendor libtiff sources to version 4.0.3.

See #12301.

comment:9 Changed 20 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [73157]) Merged libtiff 4.0.3 changes into the trunk.

This should help with making libtiff work better in 64 bit builds as libtiff 4
is supposed to support them much better.

Closes #12301.

comment:10 Changed 20 months ago by VZ

(In [73159]) Readded tiffconf.h removed by libtiff 4.0.3 merge.

This file is needed by Windows builds.

Should have been part of r73157, see #12301.

comment:11 Changed 19 months ago by ghostvoodooman

I accidentally created a dupe ( see #12938 , I maybe didn't chosen right keywords to search before creating ticket), now it is fixed.

Note: See TracTickets for help on using tickets.