Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 571654 - media-libs/libafterimage-1.20: build fails against >=media-libs/giflib-5.0 due to incompatible API changes
Summary: media-libs/libafterimage-1.20: build fails against >=media-libs/giflib-5.0 du...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: [OLD] Development (show other bugs)
Hardware: All All
: Normal normal with 1 vote (vote)
Assignee: Gentoo Science Physics related packages
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: giflib-5
  Show dependency tree
 
Reported: 2016-01-12 13:27 UTC by Guilherme Amadio
Modified: 2017-01-17 22:43 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
use-bundled-giflib.patch (use-bundled-giflib.patch,1.58 KB, patch)
2016-01-12 18:36 UTC, Guilherme Amadio
Details | Diff
emerge-info.txt (emerge-info.txt,6.55 KB, text/plain)
2016-04-28 08:19 UTC, Toralf Förster
Details
emerge-history.txt (emerge-history.txt,283.46 KB, text/plain)
2016-04-28 08:19 UTC, Toralf Förster
Details
environment (environment,101.55 KB, text/plain)
2016-04-28 08:19 UTC, Toralf Förster
Details
media-libs:libafterimage-1.20:20160428-060150.log (media-libs:libafterimage-1.20:20160428-060150.log,22.28 KB, text/plain)
2016-04-28 08:19 UTC, Toralf Förster
Details
Patch to fix implicit bad cast, causing segmentation fault (libafterimage-giflib5.patch,10.88 KB, patch)
2017-01-17 15:34 UTC, Oliver Freyermuth
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Guilherme Amadio gentoo-dev 2016-01-12 13:27:16 UTC
API changes to make library reentrant introduced parameters into file open/close functions. More information in the link below.

http://giflib.sourceforge.net/gif_lib.html#compatibilit

Reproducible: Always
Comment 1 Guilherme Amadio gentoo-dev 2016-01-12 13:30:39 UTC
(In reply to Guilherme Amadio from comment #0)
> API changes to make library reentrant introduced parameters into file
> open/close functions. More information in the link below.
> 
> http://giflib.sourceforge.net/gif_lib.html#compatibilit
> 
> Reproducible: Always

Correction to the link:

http://giflib.sourceforge.net/gif_lib.html#compatibility
Comment 2 Juergen Rose 2016-01-12 14:11:50 UTC
After installing giflib-5.1.2 'emerge libafterimage' fails here with:

x86_64-pc-linux-gnu-gcc -DNO_DEBUG_OUTPUT -march=native -O2 -pipe -fPIC -Wall -DNO_DEBUG_OUTPUT -march=native -pipe -fPIC -Wall  -mmmx -Winline --param inline-unit-growth=10000 --param large-function-growth=10000  -pthread -I/usr/include/librsvg-2.0 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libdrm -I/usr/include/libpng16  -I/usr/include/freetype2    -c bmp.c -o bmp.o
asstorage.c: In function ‘compress_stored_data’:
asstorage.c:719:10: warning: variable ‘buf_size’ set but not used [-Wunused-but-set-variable]
  size_t  buf_size = size ; 
          ^
In file included from afterbase.h:36:0,
                 from ascmap.c:56:
ascmap.c: In function ‘colormap_asimage’:
ascmap.c:413:13: warning: variable ‘started’ set but not used [-Wunused-but-set-variable]
  START_TIME(started);
             ^
Comment 3 Toralf Förster gentoo-dev 2016-01-12 14:53:49 UTC
And here :#

ungif.h: In function ‘PrintGifError’:
ungif.h:12:29: error: too few arguments to function ‘GifErrorString’
     fprintf(stderr, "%s\n", GifErrorString());
                             ^
In file included from export.c:91:0:
/usr/include/gif_lib.h:228:20: note: declared here
Comment 4 Guilherme Amadio gentoo-dev 2016-01-12 15:03:17 UTC
(In reply to Juergen Rose from comment #2)
> After installing giflib-5.1.2 'emerge libafterimage' fails here with:
> 
> x86_64-pc-linux-gnu-gcc -DNO_DEBUG_OUTPUT -march=native -O2 -pipe -fPIC
> -Wall -DNO_DEBUG_OUTPUT -march=native -pipe -fPIC -Wall  -mmmx -Winline
> --param inline-unit-growth=10000 --param large-function-growth=10000 
> -pthread -I/usr/include/librsvg-2.0 -I/usr/include/gdk-pixbuf-2.0
> -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include
> -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libdrm
> -I/usr/include/libpng16  -I/usr/include/freetype2    -c bmp.c -o bmp.o
> asstorage.c: In function ‘compress_stored_data’:
> asstorage.c:719:10: warning: variable ‘buf_size’ set but not used
> [-Wunused-but-set-variable]
>   size_t  buf_size = size ; 
>           ^
> In file included from afterbase.h:36:0,
>                  from ascmap.c:56:
> ascmap.c: In function ‘colormap_asimage’:
> ascmap.c:413:13: warning: variable ‘started’ set but not used
> [-Wunused-but-set-variable]
>   START_TIME(started);
>              ^

Those are warnings, the real problem is the change in the API as shown in comment #3 (number of arguments to file open/close functions and error handling functions).
Comment 5 Guilherme Amadio gentoo-dev 2016-01-12 18:36:45 UTC
Created attachment 422732 [details, diff]
use-bundled-giflib.patch

This is a workaround that switches to using the bundled giflib.
Upstream seems dead (last update in 2010) and only sci-physics/root and x11-wm/afterstep depend on media-libs/libafterimage.
Comment 6 Andrew Savchenko gentoo-dev 2016-01-12 21:38:04 UTC
(In reply to Guilherme Amadio from comment #5)
> Created attachment 422732 [details, diff] [details, diff]
> use-bundled-giflib.patch

No, this is a bad idea. The same way we can use bundled afterimage in root (and sometimes we had to due to segfaults with external version). But people will yell tons of reports if we do. And well, unbundling policy do exists for a reason.

Actually we had similar problem earlier: a port to giflib-4 API. Change was rather simple, see libafterimage-giflib42.patch.

> x11-wm/afterstep depend on media-libs/libafterimage

No, it hardblocks with it:
!!media-libs/libafterimage

So we have only one real dependency: root. We should either decide to use bundled afterimage there or continue to support rotting libafterimage properly.
Comment 7 Guilherme Amadio gentoo-dev 2016-01-13 01:54:52 UTC
(In reply to Andrew Savchenko from comment #6)
> (In reply to Guilherme Amadio from comment #5)
> > Created attachment 422732 [details, diff] [details, diff] [details, diff]
> > use-bundled-giflib.patch
> 
> No, this is a bad idea. The same way we can use bundled afterimage in root
> (and sometimes we had to due to segfaults with external version). But people
> will yell tons of reports if we do. And well, unbundling policy do exists
> for a reason.
> 
> Actually we had similar problem earlier: a port to giflib-4 API. Change was
> rather simple, see libafterimage-giflib42.patch.
> 
> > x11-wm/afterstep depend on media-libs/libafterimage
> 
> No, it hardblocks with it:
> !!media-libs/libafterimage
> 
> So we have only one real dependency: root. We should either decide to use
> bundled afterimage there or continue to support rotting libafterimage
> properly.

I actually contacted upstream and they are working on a patch, and possibly a new release using unbundled libraries, including giflib-5 support. The patch I sent is basically a workaround if people need to compile but can't just yet because they must have giflib-5.1 in their system due to other deps.

I will update this bug accordingly when I get either a patch from upstream, or work one out myself.
Comment 8 Andrew Savchenko gentoo-dev 2016-01-13 14:28:03 UTC
Great news that project is still alive.
Comment 9 Juergen Rose 2016-01-31 15:54:29 UTC
(In reply to Andrew Savchenko from comment #8)
> Great news that project is still alive.

Any news? 'emerge -uvND world' complains for more than two weeks about broken libafterimage-1.20. Can I use the patch of Comment 5 or should I wait for a upstream solution?
Comment 10 Andrew Savchenko gentoo-dev 2016-01-31 16:14:27 UTC
You may use the patch as a temporary workaround.
Comment 11 Toralf Förster gentoo-dev 2016-04-28 08:19:41 UTC
Created attachment 432404 [details]
emerge-info.txt

In file included from export.c:111:0:
ungif.h: In function ‘PrintGifError’:
ungif.h:12:29: error: too few arguments to function ‘GifErrorString’
     fprintf(stderr, "%s\n", GifErrorString());
                             ^
In file included from export.c:91:0:
Comment 12 Toralf Förster gentoo-dev 2016-04-28 08:19:44 UTC
Created attachment 432406 [details]
emerge-history.txt
Comment 13 Toralf Förster gentoo-dev 2016-04-28 08:19:46 UTC
Created attachment 432408 [details]
environment
Comment 14 Toralf Förster gentoo-dev 2016-04-28 08:19:50 UTC
Created attachment 432410 [details]
media-libs:libafterimage-1.20:20160428-060150.log
Comment 15 Andrew Savchenko gentoo-dev 2016-04-30 10:45:11 UTC
@Guilherme, any news from upstream?

The newest commit in CVS is dated 2012-12-25.

If they are unwilling to support this library we should really consider purging it and switching root to a bundled version.
Comment 16 Andrew Savchenko gentoo-dev 2016-04-30 10:46:08 UTC
Note: host cvs.aftercode.net is dead, I used aftercode.net instead.
Comment 17 Andrew Savchenko gentoo-dev 2016-05-02 07:19:54 UTC
I peeked into the root code, unfortunately their solution is to continue use old and vulnerable libraries bundled in libafterimage bundled in the root, only libpng got some update. Thus it looks like we have to continue support of standalone libafterimage ebuild.

Right now I'm working on porting libafterimage to giflib-5 API, this is untrivial, but doable; so please be patient :)
Comment 18 Guilherme Amadio gentoo-dev 2016-05-02 12:26:41 UTC
(In reply to Andrew Savchenko from comment #15)
> @Guilherme, any news from upstream?
> 
> The newest commit in CVS is dated 2012-12-25.
> 
> If they are unwilling to support this library we should really consider
> purging it and switching root to a bundled version.

Hi Andrew, no news from upstream. I looked into the code too, and agree with you. It is not trivial to update, but also not impossible. We should consider using ROOT with bundled libraries and placing ewarn messages to the users while we don't have a ported libafterimage.

Cheers,
Comment 19 Andrew Savchenko gentoo-dev 2016-05-02 15:54:33 UTC
Support for giflib-5 is added in libafterimage-1.20-r1. I checked that root + gif works fine as well as native ascompose tool can read, display, and write gif images. Heh, quite a work was done.

Support for older giflib versions is preserved via #define checks at compile time.
Comment 20 Guilherme Amadio gentoo-dev 2016-05-02 16:22:56 UTC
(In reply to Andrew Savchenko from comment #19)
> Support for giflib-5 is added in libafterimage-1.20-r1. I checked that root
> + gif works fine as well as native ascompose tool can read, display, and
> write gif images. Heh, quite a work was done.
> 
> Support for older giflib versions is preserved via #define checks at compile
> time.

Thanks a lot for this! Should we send your patch upstream?
Comment 21 Andrew Savchenko gentoo-dev 2016-05-02 17:04:39 UTC
(In reply to Guilherme Amadio from comment #20)
> Thanks a lot for this! Should we send your patch upstream?

I just sent e-mail upstream, sci-physics is on CC.
Comment 22 Oliver Freyermuth 2017-01-17 14:10:45 UTC
Hi, 

sorry to revive this - but since I do not see this patch anywhere upstream yet, libafterimage upstream looks dead,
and I notice a crash with ROOT + Eve seemingly related to the giflib-5 upgrade and this patch, I guess I'll have to ask here. 

If I should rather open a new issue, please tell me. 

Using =sci-physics/root-5.34.36, I do:

$ root -l
root [0] TEveManager::Create(kTRUE, "FI")

immediate crash, stacktrace with debug symbols:
===========================================================
#6  GifFreeExtensions (ExtensionBlockCount=0x2, ExtensionBlocks=0x2c1e160) at /usr/src/debug/media-libs/giflib-5.1.4/giflib-5.1.4/lib/gifalloc.c:271
#7  0x00007f3aaea98e3e in free_gif_saved_image (sp=sp
entry=0x7fff638764f0, reusable=reusable
entry=1) at ungif.c:89
#8  0x00007f3aaea99110 in get_gif_saved_images (gif=gif
entry=0x2bfb0d0, subimage=0, ret=ret
entry=0x7fff638765f0, ret_images=ret_images
entry=0x7fff638765ec) at ungif.c:255
#9  0x00007f3aaea82869 in gif2ASImage (path=0x2c1dea0 "/usr/share/root/icons/eve_text.gif", params=0x7f3aaeef0040 <TASImage::ReadImage(char const*, TImage::EImageFileTypes)::iparams>) at import.c:2201
#10 0x00007f3aaea8038a in file2ASImage_extra (file=<optimized out>, iparams=0x7f3aaeef0040 <TASImage::ReadImage(char const*, TImage::EImageFileTypes)::iparams>) at import.c:275
#11 0x00007f3aaecc9e6e in TASImage::ReadImage (this=0x2c1dd70, filename=0x2c1de70 "/usr/share/root/icons/eve_text.gif") at /usr/src/debug/sci-physics/root-5.34.36/root/graf2d/asimage/src/TASImage.cxx:541
#12 0x00007f3abbe97a60 in TImage::Open (file=file
entry=0x2c1dcf0 "/usr/share/root/icons/eve_text.gif", type=type
entry=TImage::kUnknown) at /usr/src/debug/sci-physics/root-5.34.36/root/graf2d/graf/src/TImage.cxx:120
#13 0x00007f3abce95053 in TGPicturePool::GetPicture (this=0x296f300, name=name
entry=0x7f3ab88b544c "eve_text.gif") at /usr/src/debug/sci-physics/root-5.34.36/root/gui/gui/src/TGPicture.cxx:111
#14 0x00007f3abcf4243c in TGClient::GetPicture (this=<optimized out>, name=name
entry=0x7f3ab88b544c "eve_text.gif") at /usr/src/debug/sci-physics/root-5.34.36/root/gui/gui/src/TGClient.cxx:263
#15 0x00007f3ab8723c85 in TEveUtil::SetupGUI () at /usr/src/debug/sci-physics/root-5.34.36/root/graf3d/eve/src/TEveUtil.cxx:112
#16 0x00007f3ab871b1df in TEveManager::Create (map_window=<optimized out>, opt=0x2ab98b8 "FI") at /usr/src/debug/sci-physics/root-5.34.36/root/graf3d/eve/src/TEveManager.cxx:964
===========================================================
Comment 23 Oliver Freyermuth 2017-01-17 15:29:04 UTC
Sorry to write again, looks like a bug in the patch, below an excerpt from the compile log. This implicit cast is indeed very wrong. I'll try to cook up an updated patch...

ungif.c: In function ‘free_gif_saved_image’:
ungif.c:89:25: warning: passing argument 1 of ‘GifFreeExtensions’ makes pointer from integer without a cast
       GifFreeExtensions(sp->ExtensionBlockCount, sp->ExtensionBlocks);
                         ^
In file included from ungif.c:62:0:
/usr/include/gif_lib.h:259:13: note: expected ‘int *’ but argument is of type ‘int’
 extern void GifFreeExtensions(int *ExtensionBlock_Count,
             ^
ungif.c:89:50: warning: passing argument 2 of ‘GifFreeExtensions’ from incompatible pointer type
       GifFreeExtensions(sp->ExtensionBlockCount, sp->ExtensionBlocks);
                                                  ^
In file included from ungif.c:62:0:
/usr/include/gif_lib.h:259:13: note: expected ‘struct ExtensionBlock **’ but argument is of type ‘struct ExtensionBlock *’
 extern void GifFreeExtensions(int *ExtensionBlock_Count,
             ^
Comment 24 Oliver Freyermuth 2017-01-17 15:34:30 UTC
Created attachment 460466 [details, diff]
Patch to fix implicit bad cast, causing segmentation fault

Using the attached patch instead of the in-tree libafterimage-giflib5.patch fixes the issue. 
I just added the two missing "&" at the right place. 

Running ROOT with event-display, I still get:
gif2ASImage():2300:</usr/share/root/icons/eve_text.gif> (null)
But there seem to be no further issues (maybe this icon is missing somewhere in the GUI, but I do not see it...).
Comment 25 Andrew Savchenko gentoo-dev 2017-01-17 22:43:56 UTC
Hi,

(In reply to Oliver Freyermuth from comment #22)
> Hi, 
> 
> sorry to revive this - but since I do not see this patch anywhere upstream
> yet, libafterimage upstream looks dead,

Yes, upstream is VERY dead. I send them two e-mails at May 2016, no reply; no other contacts, bugzilla or whatever.

Unfortunately we have to support it, because the only alternative is to use root with bundled libafterimage with bundled libungif, which is very old and vulnerable. So we'll have a storm of unbundling bugs and security team frowns.

> and I notice a crash with ROOT + Eve seemingly related to the giflib-5
> upgrade and this patch, I guess I'll have to ask here. 
> 
> If I should rather open a new issue, please tell me. 

It's OK for now, since all necessary information is provided and I don't want to torture you with unnecessary bureaucracy :). But for any future problems with any package please open a separate bug for each separate issue. This way it will be easier for other users to find this bug as well as for developers to keep track on remaining issues. While both problems are connected to giflib-5 and libafterimage, the first one about support for giflib-5 and the second one is about bug in this support.

(In reply to Oliver Freyermuth from comment #24)
> Created attachment 460466 [details, diff] [details, diff]
> Patch to fix implicit bad cast, causing segmentation fault

Patch looks OK, applied in libafterimage-1.20-r2 together with another pointer related fix for signedness problem.