* Problem with library images on Windows (again) @ 2005-05-18 15:53 Juanma Barranquero 2005-05-18 19:30 ` Andreas Schwab 2005-05-18 22:16 ` Jason Rumney 0 siblings, 2 replies; 37+ messages in thread From: Juanma Barranquero @ 2005-05-18 15:53 UTC (permalink / raw) For some time now, TIFF images do crash optimized MSVC builds of Emacs when using the MinGW-supplied TIFF library. The crash happens in image_spec_value(), but it all boils down to the fact that on lookup_image(), after img->load_failed_p = img->type->load (f, img) == 0; the value of the argument "spec" cannot be trusted. This is easy to prove; with static LispObject spec_backup; spec_backup = spec; img->load_failed_p = img->type->load (f, img) == 0; spec = spec_backup; spec_backup = Qnil; everything works as expected. Now, when this happened before (with the local variable img) someone said that the MinGW libraries were not respecting calling conventions, but I'm not sure. Calling conventions for MSVC say that you cannot trust EAX, EBX, ECX, EDX, ESI, or EDI registers after a subroutine call, and AFAICS, the problem is that on optimized builds VC is expecting spec to still be cached in a register. So, I can apply the attached patch and at least stop Emacs from crashing on Windows, but I certainly would appreciate a more insightful analysis from the "people in the know" wrt image support and/or Windows. -- /L/e/k/t/u Index: src/image.c =================================================================== RCS file: /cvsroot/emacs/emacs/src/image.c,v retrieving revision 1.23 diff -u -2 -r1.23 image.c --- src/image.c 10 May 2005 09:16:46 -0000 1.23 +++ src/image.c 18 May 2005 15:38:22 -0000 @@ -1663,9 +1663,20 @@ extern Lisp_Object Qpostscript; +#ifdef _MSC_VER + static Lisp_Object spec_backup; + spec_backup = spec; +#endif + BLOCK_INPUT; img = make_image (spec, hash); cache_image (f, img); + img->load_failed_p = img->type->load (f, img) == 0; +#ifdef _MSC_VER + spec = spec_backup; + spec_backup = Qnil; +#endif + /* If we can't load the image, and we don't have a width and height, use some arbitrary width and height so that we can ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-18 15:53 Problem with library images on Windows (again) Juanma Barranquero @ 2005-05-18 19:30 ` Andreas Schwab 2005-05-18 22:44 ` Juanma Barranquero 2005-05-18 22:16 ` Jason Rumney 1 sibling, 1 reply; 37+ messages in thread From: Andreas Schwab @ 2005-05-18 19:30 UTC (permalink / raw) Cc: emacs-devel Juanma Barranquero <lekktu@gmail.com> writes: > Now, when this happened before (with the local variable img) someone > said that the MinGW libraries were not respecting calling conventions, > but I'm not sure. Calling conventions for MSVC say that you cannot > trust EAX, EBX, ECX, EDX, ESI, or EDI registers after a subroutine That's quite different from the Unix calling conventions on i386, where only EAX, ECX and EDX are call-clobbered. > call, and AFAICS, the problem is that on optimized builds VC is > expecting spec to still be cached in a register. That appears like a blatant bug in the compiler if it cannot properly track call-clobbered registers. Andreas. -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-18 19:30 ` Andreas Schwab @ 2005-05-18 22:44 ` Juanma Barranquero 2005-05-19 6:58 ` Jason Rumney 0 siblings, 1 reply; 37+ messages in thread From: Juanma Barranquero @ 2005-05-18 22:44 UTC (permalink / raw) > That's quite different from the Unix calling conventions on i386, where > only EAX, ECX and EDX are call-clobbered. I was wrong wrt the MSVC calling conventions. MSDN docs say: All arguments are widened to 32 bits when they are passed. Return values are also widened to 32 bits and returned in the EAX register, except for 8-byte structures, which are returned in the EDX:EAX register pair. Larger structures are returned in the EAX register as pointers to hidden return structures. Parameters are pushed onto the stack from right to left. The compiler generates prolog and epilog code to save and restore the ESI, EDI, EBX, and EBP registers, if they are used in the function. so it seems it's the same calling convention than on i386 Unixes: EAX and EDX can be clobbered to return the function's result, and ECX because it is not saved. > That appears like a blatant bug in the compiler if it cannot properly > track call-clobbered registers. There's a bug somewhere, that's pretty clear. Whether that's an error of the MSVC compiler or the MinGW libraries, I'm not sure yet. I've been tracing lookup_image, and the fact is, the lookup_image code is using EBX to cache "spec", and rightly so. But the call to tiff_load (via img->type->load()) clobbers EBX! -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-18 22:44 ` Juanma Barranquero @ 2005-05-19 6:58 ` Jason Rumney 0 siblings, 0 replies; 37+ messages in thread From: Jason Rumney @ 2005-05-19 6:58 UTC (permalink / raw) Cc: emacs-devel Juanma Barranquero <lekktu@gmail.com> writes: >> That appears like a blatant bug in the compiler if it cannot properly >> track call-clobbered registers. > > There's a bug somewhere, that's pretty clear. Whether that's an error > of the MSVC compiler or the MinGW libraries, I'm not sure yet. FWIW the problem with the jpeg library was that the headers defined some constant like BOOL or BYTE differently than the MSVC headers, so it ended up a different size. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-18 15:53 Problem with library images on Windows (again) Juanma Barranquero 2005-05-18 19:30 ` Andreas Schwab @ 2005-05-18 22:16 ` Jason Rumney 2005-05-18 22:52 ` Juanma Barranquero 1 sibling, 1 reply; 37+ messages in thread From: Jason Rumney @ 2005-05-18 22:16 UTC (permalink / raw) Cc: emacs-devel Juanma Barranquero <lekktu@gmail.com> writes: > > So, I can apply the attached patch and at least stop Emacs from > crashing on Windows, but I certainly would appreciate a more > insightful analysis from the "people in the know" wrt image support > and/or Windows. Better to disable image support for MSVC builds than to try to hack things to work by introducing redundant variables to save the values of other variables in. Such hacks might be worth it to support free software where we can see whether the hack really does fix the problem, but not for proprietary blackboxes. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-18 22:16 ` Jason Rumney @ 2005-05-18 22:52 ` Juanma Barranquero 2005-05-19 3:50 ` Eli Zaretskii 0 siblings, 1 reply; 37+ messages in thread From: Juanma Barranquero @ 2005-05-18 22:52 UTC (permalink / raw) > Better to disable image support for MSVC builds than to try to hack > things to work by introducing redundant variables to save the values > of other variables in. Perhaps; note that I didn't say it wasn't a hack. OTOH, I normally use a MSVC build and certainly don't appreciate Emacs crashing on me just because I accidentally opened a TIFF file. But disabling all image support seems a bit extreme. Anyway: tiff_load is pushing EBX into the stack and getting a different EBX when poping it out at the end of the function! I've not yet traced it to determine what or when it's changing the stack, or whether it is the MSVC-generated tiff_load code or the MinGW-built TIFF libraries that are called on several spots of tiff_load, but in any case something's afoot (as Sherlock Holmes would say). -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-18 22:52 ` Juanma Barranquero @ 2005-05-19 3:50 ` Eli Zaretskii 2005-05-19 8:13 ` Juanma Barranquero 0 siblings, 1 reply; 37+ messages in thread From: Eli Zaretskii @ 2005-05-19 3:50 UTC (permalink / raw) Cc: emacs-devel > Date: Thu, 19 May 2005 00:52:56 +0200 > From: Juanma Barranquero <lekktu@gmail.com> > > Anyway: tiff_load is pushing EBX into the stack and getting a > different EBX when poping it out at the end of the function! Can you set a watchpoint on the address where EBX is saved on the stack, and then see what code clobbers that value? ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-19 3:50 ` Eli Zaretskii @ 2005-05-19 8:13 ` Juanma Barranquero 2005-05-19 19:05 ` Eli Zaretskii 0 siblings, 1 reply; 37+ messages in thread From: Juanma Barranquero @ 2005-05-19 8:13 UTC (permalink / raw) On 5/19/05, Eli Zaretskii <eliz@gnu.org> wrote: > Can you set a watchpoint on the address where EBX is saved on the > stack, and then see what code clobbers that value? Yes. tiff_load pushes EBX into the stack, and then goes around happily pushing arguments and calling functions from the TIFF library, which apparently don't remove their arguments from the stack (nor does it the tiff_load code). So the stack keeps growing and, by the time tiff_load does the POP EBX, it gets random garbage. On returning from tiff_load the frame pointer is restored, so the spurious stack data is just discarded. So it seems like a mismatch between the calling convention used to implement the TIFF library and the one MSVC thinks the TIFF functions are using. I'll dig around a little more. But any help is much appreciated. -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-19 8:13 ` Juanma Barranquero @ 2005-05-19 19:05 ` Eli Zaretskii 2005-05-19 19:31 ` Juanma Barranquero 0 siblings, 1 reply; 37+ messages in thread From: Eli Zaretskii @ 2005-05-19 19:05 UTC (permalink / raw) Cc: emacs-devel > Date: Thu, 19 May 2005 10:13:23 +0200 > From: Juanma Barranquero <lekktu@gmail.com> > > tiff_load pushes EBX into the stack, and then goes around happily > pushing arguments and calling functions from the TIFF library, which > apparently don't remove their arguments from the stack (nor does it > the tiff_load code). So the stack keeps growing This sounds incredible. MSVC generally produces good code, so I find it hard to believe that it violates the C ABI, whereby the caller should pop arguments off the stack. Are you sure this is indeed what happens? Can you show some evidence, like disassembly of code around the call to tiff_load, which would show how arguments are pushed onto the stack and never popped off it? ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-19 19:05 ` Eli Zaretskii @ 2005-05-19 19:31 ` Juanma Barranquero 2005-05-19 19:32 ` Juanma Barranquero 2005-05-20 7:20 ` Eli Zaretskii 0 siblings, 2 replies; 37+ messages in thread From: Juanma Barranquero @ 2005-05-19 19:31 UTC (permalink / raw) > This sounds incredible. MSVC generally produces good code, so I find > it hard to believe that it violates the C ABI, whereby the caller > should pop arguments off the stack. Are you sure this is indeed what > happens? Yes. If, immediately before the POP EBX, I manually restore the stack to the value it had after the PUSH EBX at the start of the routine, Emacs does not crash, and indeed it displays the TIFF file. But I don't think is an error in MSVC. I think it's some mixup between "pascal" and "C" style calls (as you know, the former remove their arguments, the latter don't). MSVC must be thinking that the external TIFF functions do remove their arguments, so it is not doing it. Sounds like an issue with the external declarations on the header files. > Can you show some evidence, like disassembly of code around > the call to tiff_load, which would show how arguments are pushed onto > the stack and never popped off it? There goes the full tiff_load code, as shown on the Visual Studio disassembler view. You can see by yourself that, when calling Emacs functions, MSVC uses "ADD SP, nn" to fix the stack, but it doesn't do it after the indirect calls to fn_TIFF* functions. -- /L/e/k/t/u ---------------------------------------------------------------------------------------- /* Load TIFF image IMG for use on frame F. Value is non-zero if successful. */ static int tiff_load (f, img) struct frame *f; struct image *img; { 01074117 push ebp 01074118 mov ebp,esp 0107411A sub esp,24h 0107411D push ebx Lisp_Object file, specified_file; Lisp_Object specified_data; TIFF *tiff; int width, height, x, y; uint32 *buf; int rc; XImagePtr ximg; struct gcpro gcpro1; tiff_memory_source memsrc; specified_file = image_spec_value (img->spec, QCfile, NULL); 0107411E mov ebx,dword ptr [ebp+0Ch] 01074121 mov ecx,dword ptr [ebx+28h] 01074124 push esi 01074125 push edi 01074126 xor edi,edi 01074128 push edi 01074129 push dword ptr [_QCfile (1109E80h)] 0107412F call image_spec_value (106FF15h) specified_data = image_spec_value (img->spec, QCdata, NULL); 01074134 mov ecx,dword ptr [ebx+28h] 01074137 push edi 01074138 push dword ptr [_QCdata (1109064h)] 0107413E mov dword ptr [specified_file],eax 01074141 call image_spec_value (106FF15h) 01074146 add esp,10h file = Qnil; GCPRO1 (file); fn_TIFFSetErrorHandler (tiff_error_handler); 01074149 push offset tiff_error_handler (10723B8h) 0107414E mov esi,eax 01074150 call dword ptr [_fn_TIFFSetErrorHandler (1138B64h)] fn_TIFFSetWarningHandler (tiff_warning_handler); 01074156 push offset tiff_warning_handler (1072400h) 0107415B call dword ptr [_fn_TIFFSetWarningHandler (1138B78h)] if (NILP (specified_data)) 01074161 cmp esi,dword ptr [_Qnil (1108D6Ch)] 01074167 jne tiff_load+0AEh (10741C5h) { /* Read from a file */ file = x_find_image_file (specified_file); 01074169 push dword ptr [specified_file] 0107416C call x_find_image_file (10706E5h) 01074171 pop ecx if (!STRINGP (file)) 01074172 mov ecx,eax 01074174 and ecx,0E0000000h 0107417A cmp ecx,60000000h 01074180 mov dword ptr [buf],eax 01074183 je tiff_load+81h (1074198h) { image_error ("Cannot find image file `%s'", specified_file, Qnil); 01074185 push dword ptr [_Qnil (1108D6Ch)] 0107418B push dword ptr [specified_file] 0107418E push offset string "Cannot find image file `%s'" (1166718h) UNGCPRO; return 0; 01074193 jmp tiff_load+11Ah (1074231h) } /* Try to open the image file. */ tiff = fn_TIFFOpen (SDATA (file), "r"); 01074198 push offset string "r" (115F1E0h) 0107419D and eax,1FFFFFFFh 010741A2 push dword ptr [eax+0Ch] 010741A5 call dword ptr [_fn_TIFFOpen (1138AC0h)] 010741AB mov esi,eax if (tiff == NULL) 010741AD cmp esi,edi 010741AF jne tiff_load+129h (1074240h) { image_error ("Cannot open `%s'", file, Qnil); 010741B5 push dword ptr [_Qnil (1108D6Ch)] 010741BB push dword ptr [buf] 010741BE push offset string "Cannot open `%s'" (1166DA4h) UNGCPRO; return 0; 010741C3 jmp tiff_load+11Ah (1074231h) } } else { /* Memory source! */ memsrc.bytes = SDATA (specified_data); 010741C5 mov eax,esi 010741C7 and eax,1FFFFFFFh 010741CC mov ecx,dword ptr [eax+0Ch] 010741CF mov dword ptr [memsrc],ecx memsrc.len = SBYTES (specified_data); 010741D2 mov ecx,dword ptr [eax+4] 010741D5 cmp ecx,edi 010741D7 jge tiff_load+0C9h (10741E0h) 010741D9 mov eax,dword ptr [eax] 010741DB mov dword ptr [ebp-20h],eax 010741DE jmp tiff_load+0CCh (10741E3h) 010741E0 mov dword ptr [ebp-20h],ecx memsrc.index = 0; tiff = fn_TIFFClientOpen ("memory_source", "r", &memsrc, (TIFFReadWriteProc) tiff_read_from_memory, (TIFFReadWriteProc) tiff_write_from_memory, tiff_seek_in_memory, tiff_close_memory, tiff_size_of_memory, tiff_mmap_memory, tiff_unmap_memory); 010741E3 push offset tiff_unmap_memory (10723ACh) 010741E8 push offset tiff_mmap_memory (10723A9h) 010741ED push offset tiff_size_of_memory (10723ADh) 010741F2 push offset tiff_close_memory (10723A6h) 010741F7 push offset tiff_seek_in_memory (107236Fh) 010741FC push offset tiff_write_from_memory (107236Bh) 01074201 push offset tiff_read_from_memory (1072335h) 01074206 lea eax,[memsrc] 01074209 push eax 0107420A push offset string "r" (115F1E0h) 0107420F push offset string "memory_source" (1166DFCh) 01074214 mov dword ptr [ebp-1Ch],edi 01074217 call dword ptr [_fn_TIFFClientOpen (1138B00h)] 0107421D mov esi,eax if (!tiff) 0107421F cmp esi,edi 01074221 jne tiff_load+129h (1074240h) { image_error ("Cannot open memory source for `%s'", img->spec, Qnil); 01074223 push dword ptr [_Qnil (1108D6Ch)] 01074229 push dword ptr [ebx+28h] 0107422C push offset string "Cannot open memory source for `%"... (1166DD8h) 01074231 call add_to_log (1035C57h) 01074236 add esp,0Ch UNGCPRO; return 0; 01074239 xor eax,eax 0107423B jmp tiff_load+272h (1074389h) } } /* Get width and height of the image, and allocate a raster buffer of width x height 32-bit values. */ fn_TIFFGetField (tiff, TIFFTAG_IMAGEWIDTH, &width); 01074240 lea eax,[width] 01074243 push eax 01074244 push 100h 01074249 push esi 0107424A call dword ptr [_fn_TIFFGetField (1138B2Ch)] fn_TIFFGetField (tiff, TIFFTAG_IMAGELENGTH, &height); 01074250 lea eax,[height] 01074253 push eax 01074254 push 101h 01074259 push esi 0107425A call dword ptr [_fn_TIFFGetField (1138B2Ch)] buf = (uint32 *) xmalloc (width * height * sizeof *buf); 01074260 mov eax,dword ptr [height] 01074263 imul eax,dword ptr [width] 01074267 shl eax,2 0107426A push eax 0107426B call xmalloc (10070AFh) 01074270 pop ecx rc = fn_TIFFReadRGBAImage (tiff, width, height, buf, 0); 01074271 push edi 01074272 push eax 01074273 push dword ptr [height] 01074276 mov dword ptr [buf],eax 01074279 push dword ptr [width] 0107427C push esi 0107427D call dword ptr [_fn_TIFFReadRGBAImage (1138B90h)] fn_TIFFClose (tiff); 01074283 push esi 01074284 mov dword ptr [specified_file],eax 01074287 call dword ptr [_fn_TIFFClose (1138AECh)] if (!rc) 0107428D cmp dword ptr [specified_file],edi 01074290 jne tiff_load+198h (10742AFh) { image_error ("Error reading TIFF image `%s'", img->spec, Qnil); 01074292 push dword ptr [_Qnil (1108D6Ch)] 01074298 push dword ptr [ebx+28h] 0107429B push offset string "Error reading TIFF image `%s'" (1166DB8h) 010742A0 call add_to_log (1035C57h) 010742A5 add esp,0Ch 010742A8 xor esi,esi 010742AA jmp tiff_load+267h (107437Eh) xfree (buf); UNGCPRO; return 0; } /* Create the X image and pixmap. */ if (!x_create_x_image_and_pixmap (f, width, height, 0, &ximg, &img->pixmap)) 010742AF lea eax,[ebx+4] 010742B2 push eax 010742B3 push dword ptr [height] 010742B6 lea esi,[specified_file] 010742B9 push dword ptr [width] 010742BC xor eax,eax 010742BE push dword ptr [f] 010742C1 call x_create_x_image_and_pixmap (1072AE6h) 010742C6 add esp,10h 010742C9 test eax,eax 010742CB je tiff_load+191h (10742A8h) { xfree (buf); UNGCPRO; return 0; } /* Initialize the color table. */ init_color_table (); /* Process the pixel raster. Origin is in the lower-left corner. */ for (y = 0; y < height; ++y) 010742CD mov dword ptr [y],edi 010742D0 mov edi,dword ptr [height] 010742D3 test edi,edi 010742D5 jle tiff_load+224h (107433Bh) { uint32 *row = buf + y * width; 010742D7 mov eax,dword ptr [y] 010742DA imul eax,dword ptr [width] 010742DE mov ecx,dword ptr [buf] for (x = 0; x < width; ++x) 010742E1 xor esi,esi 010742E3 cmp dword ptr [width],esi 010742E6 lea eax,[ecx+eax*4] 010742E9 mov dword ptr [row],eax 010742EC jle tiff_load+21Ch (1074333h) { uint32 abgr = row[x]; 010742EE mov eax,dword ptr [row] 010742F1 mov eax,dword ptr [eax+esi*4] int r = TIFFGetR (abgr) << 8; int g = TIFFGetG (abgr) << 8; int b = TIFFGetB (abgr) << 8; XPutPixel (ximg, x, height - 1 - y, lookup_rgb_color (f, r, g, b)); 010742F4 mov ecx,eax 010742F6 shr ecx,10h 010742F9 xor edx,edx 010742FB mov dh,cl 010742FD mov ecx,eax 010742FF shr ecx,8 01074302 xor ebx,ebx 01074304 mov bh,cl 01074306 xor ecx,ecx 01074308 mov ch,al 0107430A mov eax,edx 0107430C push ecx 0107430D mov ecx,ebx 0107430F call lookup_rgb_color (10718CDh) 01074314 sub edi,dword ptr [y] 01074317 mov ebx,eax 01074319 dec edi 0107431A push edi 0107431B mov edi,dword ptr [specified_file] 0107431E push esi 0107431F call XPutPixel (10719CBh) 01074324 mov edi,dword ptr [height] 01074327 add esp,0Ch 0107432A inc esi 0107432B cmp esi,dword ptr [width] 0107432E jl tiff_load+1D7h (10742EEh) for (x = 0; x < width; ++x) 01074330 mov ebx,dword ptr [img] { xfree (buf); UNGCPRO; return 0; } /* Initialize the color table. */ init_color_table (); /* Process the pixel raster. Origin is in the lower-left corner. */ for (y = 0; y < height; ++y) 01074333 inc dword ptr [y] 01074336 cmp dword ptr [y],edi 01074339 jl tiff_load+1C0h (10742D7h) } } #ifdef COLOR_TABLE_SUPPORT /* Remember the colors allocated for the image. Free the color table. */ img->colors = colors_in_color_table (&img->ncolors); free_color_table (); #endif /* COLOR_TABLE_SUPPORT */ img->width = width; 0107433B mov eax,dword ptr [width] img->height = height; /* Maybe fill in the background field while we have ximg handy. */ if (NILP (image_spec_value (img->spec, QCbackground, NULL))) 0107433E mov ecx,dword ptr [ebx+28h] 01074341 mov dword ptr [ebx+1Ch],eax 01074344 mov dword ptr [ebx+20h],edi 01074347 push 0 01074349 push dword ptr [_QCbackground (1141828h)] 0107434F call image_spec_value (106FF15h) 01074354 cmp eax,dword ptr [_Qnil (1108D6Ch)] 0107435A pop ecx 0107435B pop ecx 0107435C jne tiff_load+25Ch (1074373h) IMAGE_BACKGROUND (img, f, ximg); 0107435E test byte ptr [ebx+18h],2 01074362 jne tiff_load+25Ch (1074373h) 01074364 push dword ptr [specified_file] 01074367 push dword ptr [f] 0107436A push ebx 0107436B call image_background (107012Ah) 01074370 add esp,0Ch /* Put the image into the pixmap, then free the X image and its buffer. */ x_put_x_image (f, ximg, img->pixmap, width, height); x_destroy_x_image (ximg); 01074373 mov eax,dword ptr [specified_file] 01074376 call x_destroy_x_image (10706D5h) 0107437B xor esi,esi 0107437D inc esi xfree (buf); 0107437E push dword ptr [buf] 01074381 call xfree (1007174h) 01074386 pop ecx UNGCPRO; return 1; 01074387 mov eax,esi 01074389 pop edi 0107438A pop esi 0107438B pop ebx } 0107438C leave 0107438D ret ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-19 19:31 ` Juanma Barranquero @ 2005-05-19 19:32 ` Juanma Barranquero 2005-05-20 7:20 ` Eli Zaretskii 1 sibling, 0 replies; 37+ messages in thread From: Juanma Barranquero @ 2005-05-19 19:32 UTC (permalink / raw) > Yes. If, immediately before the POP EBX, I manually restore the stack > to the value it had after the PUSH EBX at the start of the routine, I meant: "Immediately before POP EDI, to the value it had after PUSH EDI." -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-19 19:31 ` Juanma Barranquero 2005-05-19 19:32 ` Juanma Barranquero @ 2005-05-20 7:20 ` Eli Zaretskii 2005-05-20 8:24 ` Kim F. Storm 1 sibling, 1 reply; 37+ messages in thread From: Eli Zaretskii @ 2005-05-20 7:20 UTC (permalink / raw) Cc: emacs-devel > Date: Thu, 19 May 2005 21:31:23 +0200 > From: Juanma Barranquero <lekktu@gmail.com> > > But I don't think is an error in MSVC. I think it's some mixup between > "pascal" and "C" style calls (as you know, the former remove their > arguments, the latter don't). MSVC must be thinking that the external > TIFF functions do remove their arguments, so it is not doing it. > Sounds like an issue with the external declarations on the header > files. Can you show the declarations of the TIFF functions as they appear in the header files visible to MSVC when it compiles Emacs? ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 7:20 ` Eli Zaretskii @ 2005-05-20 8:24 ` Kim F. Storm 2005-05-20 10:01 ` Eli Zaretskii 2005-05-20 10:16 ` Jason Rumney 0 siblings, 2 replies; 37+ messages in thread From: Kim F. Storm @ 2005-05-20 8:24 UTC (permalink / raw) Cc: Juanma Barranquero, emacs-devel "Eli Zaretskii" <eliz@gnu.org> writes: >> Date: Thu, 19 May 2005 21:31:23 +0200 >> From: Juanma Barranquero <lekktu@gmail.com> >> >> But I don't think is an error in MSVC. I think it's some mixup between >> "pascal" and "C" style calls (as you know, the former remove their >> arguments, the latter don't). MSVC must be thinking that the external >> TIFF functions do remove their arguments, so it is not doing it. >> Sounds like an issue with the external declarations on the header >> files. > > Can you show the declarations of the TIFF functions as they appear in > the header files visible to MSVC when it compiles Emacs? Could it be that the following define need to specify "Pascal" somewhere for the tiff library? #define DEF_IMGLIB_FN(func) FARPROC fn_##func -- Kim F. Storm <storm@cua.dk> http://www.cua.dk ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 8:24 ` Kim F. Storm @ 2005-05-20 10:01 ` Eli Zaretskii 2005-05-20 10:16 ` Jason Rumney 1 sibling, 0 replies; 37+ messages in thread From: Eli Zaretskii @ 2005-05-20 10:01 UTC (permalink / raw) Cc: lekktu, emacs-devel > Cc: Juanma Barranquero <lekktu@gmail.com>, emacs-devel@gnu.org > From: storm@cua.dk (Kim F. Storm) > Date: Fri, 20 May 2005 10:24:55 +0200 > > Could it be that the following define need to specify "Pascal" somewhere > for the tiff library? > > #define DEF_IMGLIB_FN(func) FARPROC fn_##func I think it should be something else. First, from Juanma's investigations it looks like MSVC already thinks those functions use the Pascal ABI. Second, the MinGW compiler does produce correct code, so the fix should be only for MSVC (perhaps some preprocessor macro works for GCC, but not MSVC). And third, I think the right qualifiers are __stdcall vs __cdecl, not "Pascal". I asked to see the relevant fragments from libtiff headers used during the compilation so that we could figure out what goes wrong there. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 8:24 ` Kim F. Storm 2005-05-20 10:01 ` Eli Zaretskii @ 2005-05-20 10:16 ` Jason Rumney 2005-05-20 10:42 ` Juanma Barranquero 2005-05-20 12:19 ` Benjamin Riefenstahl 1 sibling, 2 replies; 37+ messages in thread From: Jason Rumney @ 2005-05-20 10:16 UTC (permalink / raw) Cc: Juanma Barranquero, Eli Zaretskii, emacs-devel Kim F. Storm wrote: >"Eli Zaretskii" <eliz@gnu.org> writes: > > > >>Can you show the declarations of the TIFF functions as they appear in >>the header files visible to MSVC when it compiles Emacs? >> >> > >Could it be that the following define need to specify "Pascal" somewhere >for the tiff library? > >#define DEF_IMGLIB_FN(func) FARPROC fn_##func > > The version of TIFF headers I have uses extern "C" if compiling under C++, so I doubt the library is using Pascal calling conventions. Maybe Juanma's library is different than mine. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 10:16 ` Jason Rumney @ 2005-05-20 10:42 ` Juanma Barranquero 2005-05-20 14:44 ` Eli Zaretskii 2005-05-20 12:19 ` Benjamin Riefenstahl 1 sibling, 1 reply; 37+ messages in thread From: Juanma Barranquero @ 2005-05-20 10:42 UTC (permalink / raw) > The version of TIFF headers I have uses extern "C" if compiling under > C++, so I doubt the library is using Pascal calling conventions. > Maybe Juanma's library is different than mine. Unless I'm wrong, the issue is not how the functions are defined on the headers, because we don't statically link against the functions, but how are they compiled in the library and how do we define the fn_XXX pointers to them. In this case, the GnuWin32 TIFF 3.7.2 libraries seem to be compiled to use C calling conventions, but we declare pointers to them with DEF_IMGLIB_FN, which says unequivocally that they're FARPROC (i.e, "pascal"). Defining typedef int (FAR CDECL *CFARPROC)(); #define DEF_IMGLIB_FNC(func) CFARPROC fn_##func and using DEF_IMGLIB_FNC for the Tiff functions works. The TIFF page in gnuwin32.sourceforge.net says: "The GnuWin32 distribution comes in two versions. The ordinary version uses the standard Unix equivalents, such as fopen and read, for input and output, the other version (Tiff-win32) uses the Win32 API functions, such as CreateFile and ReadFile, for input and output. The ordinary version (Tiff) is more suitable for porting Unix programs, the Win32-API version is more suitable for writing Win32 programs." However, I'm getting the same results (problems, I mean) with the standard and the Win32 versions. -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 10:42 ` Juanma Barranquero @ 2005-05-20 14:44 ` Eli Zaretskii 2005-05-20 15:46 ` Jason Rumney 2005-05-20 16:29 ` Juanma Barranquero 0 siblings, 2 replies; 37+ messages in thread From: Eli Zaretskii @ 2005-05-20 14:44 UTC (permalink / raw) Cc: emacs-devel > Date: Fri, 20 May 2005 12:42:09 +0200 > From: Juanma Barranquero <lekktu@gmail.com> > > Unless I'm wrong, the issue is not how the functions are defined on > the headers, because we don't statically link against the functions, > but how are they compiled in the library and how do we define the > fn_XXX pointers to them. It's the issue with both: the prototypes in the header files should be consistent with how the functions were compiled, since those prototypes are the only way for the compiler to know how the functions were compiled. If it doesn't know that, it might produce wrong code. > In this case, the GnuWin32 TIFF 3.7.2 > libraries seem to be compiled to use C calling conventions, but we > declare pointers to them with DEF_IMGLIB_FN, which says unequivocally > that they're FARPROC (i.e, "pascal"). That seems to be the problem. What I still don't get is how come the same declaration that uses FARPROC works for the MinGW build? Can you figure this out? If we know this, we could fix the DEF_IMGLIB_FN macro so that it does TRT for both compilers, e.g., by using __MINGW__ condition. > The TIFF page in gnuwin32.sourceforge.net says: > > "The GnuWin32 distribution comes in two versions. The ordinary version > uses the standard Unix equivalents, such as fopen and read, for input > and output, the other version (Tiff-win32) uses the Win32 API > functions, such as CreateFile and ReadFile, for input and output. The > ordinary version (Tiff) is more suitable for porting Unix programs, > the Win32-API version is more suitable for writing Win32 programs." This doesn't say anything about the calling conventions, just about the inner workings of the functions. So I'm not surprised that both versions have the same effect. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 14:44 ` Eli Zaretskii @ 2005-05-20 15:46 ` Jason Rumney 2005-05-20 15:52 ` Eli Zaretskii 2005-05-20 16:31 ` Juanma Barranquero 2005-05-20 16:29 ` Juanma Barranquero 1 sibling, 2 replies; 37+ messages in thread From: Jason Rumney @ 2005-05-20 15:46 UTC (permalink / raw) Cc: Juanma Barranquero, emacs-devel Eli Zaretskii wrote: >That seems to be the problem. What I still don't get is how come the >same declaration that uses FARPROC works for the MinGW build? Can you >figure this out? > > Without looking at the disassembly, I'd guess that MinGW gcc and non-optimised MSVC builds restore the stack pointer from a known location regardless of the calling convention, which should always work. >If we know this, we could fix the DEF_IMGLIB_FN macro so that it does >TRT for both compilers, e.g., by using __MINGW__ condition. > > I think TRT will not depend on __MINGW__. Mingw works despite there being a bug, not because of it. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 15:46 ` Jason Rumney @ 2005-05-20 15:52 ` Eli Zaretskii 2005-05-20 16:33 ` Juanma Barranquero 2005-05-20 16:31 ` Juanma Barranquero 1 sibling, 1 reply; 37+ messages in thread From: Eli Zaretskii @ 2005-05-20 15:52 UTC (permalink / raw) Cc: lekktu, emacs-devel > Date: Fri, 20 May 2005 16:46:38 +0100 > From: Jason Rumney <jasonr@gnu.org> > CC: Juanma Barranquero <lekktu@gmail.com>, emacs-devel@gnu.org > > Without looking at the disassembly, I'd guess that MinGW gcc and > non-optimised MSVC builds restore the stack pointer from a known > location regardless of the calling convention, which should always work. > > > >If we know this, we could fix the DEF_IMGLIB_FN macro so that it does > >TRT for both compilers, e.g., by using __MINGW__ condition. > > > > > I think TRT will not depend on __MINGW__. Mingw works despite there > being a bug, not because of it. So you are saying that MinGW works simply because it doesn't optimize as well as MSVC, yes? That could be it, but I'd prefer that someone looks at the code produced by MinGW and see that it indeed doesn't restore the stack pointer, either. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 15:52 ` Eli Zaretskii @ 2005-05-20 16:33 ` Juanma Barranquero 2005-05-20 17:57 ` Juanma Barranquero 0 siblings, 1 reply; 37+ messages in thread From: Juanma Barranquero @ 2005-05-20 16:33 UTC (permalink / raw) > That could be it, but I'd prefer that someone > looks at the code produced by MinGW and see that it indeed doesn't > restore the stack pointer, either. I've not had much luck trying to build with MinGW, so I won't be able to test it. Jason, could you perhaps try it? -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 16:33 ` Juanma Barranquero @ 2005-05-20 17:57 ` Juanma Barranquero 2005-05-21 10:21 ` Jason Rumney 0 siblings, 1 reply; 37+ messages in thread From: Juanma Barranquero @ 2005-05-20 17:57 UTC (permalink / raw) With the single change from #define DEF_IMGLIB_FN(func) FARPROC fn_##func to #define DEF_IMGLIB_FN(func) int (FAR CDECL *fn_##func)() all test images I have do work beautifully. MORE IMPORTANT: two horrible hacks that I added about a year ago (when I first fought this same issue and came to the wrong conclusion) to keep Emacs from crashing CAN BE REMOVED! So I'd say this is probably the right answer. I've commited the changes so people with MinGW builds can test them. If this works for them, today will be a happy day indeed :) /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 17:57 ` Juanma Barranquero @ 2005-05-21 10:21 ` Jason Rumney 2005-05-21 10:33 ` Juanma Barranquero 0 siblings, 1 reply; 37+ messages in thread From: Jason Rumney @ 2005-05-21 10:21 UTC (permalink / raw) Cc: emacs-devel Juanma Barranquero <lekktu@gmail.com> writes: > With the single change from > > #define DEF_IMGLIB_FN(func) FARPROC fn_##func > > to > > #define DEF_IMGLIB_FN(func) int (FAR CDECL *fn_##func)() > > all test images I have do work beautifully. If the other image libraries we use also use C calling conventions, then that is the correct fix, I think. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-21 10:21 ` Jason Rumney @ 2005-05-21 10:33 ` Juanma Barranquero 2005-05-21 21:13 ` Jason Rumney 0 siblings, 1 reply; 37+ messages in thread From: Juanma Barranquero @ 2005-05-21 10:33 UTC (permalink / raw) Cc: emacs-devel > If the other image libraries we use also use C calling conventions, > then that is the correct fix, I think. I confess I've not taken the time to check whether all the GnuWin32 libraries do use C calling conventions, but I've tested all kind of images with that fix, and they worked. That suggest they really do use C calling conventions; calling a C-style function as a Pascal one can get obscured by the fact that functions do reset the frame and stack pointers upon returning, but calling a Pascal-style function as a C one would remove too many things from the stack and it would crash on the next RET... However, and in the interest of peace of mind, I'll trace the functions and see that they're really doing what I think :) -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-21 10:33 ` Juanma Barranquero @ 2005-05-21 21:13 ` Jason Rumney 2005-05-22 21:21 ` Juanma Barranquero 0 siblings, 1 reply; 37+ messages in thread From: Jason Rumney @ 2005-05-21 21:13 UTC (permalink / raw) Cc: emacs-devel Juanma Barranquero <lekktu@gmail.com> writes: >> If the other image libraries we use also use C calling conventions, >> then that is the correct fix, I think. > > However, and in the interest of peace of mind, I'll trace the > functions and see that they're really doing what I think :) Studying the headers, png and jpeg explicitly use _cdecl (or __cdecl depending on compiler), while xpm and gif specify nothing (so will default to _cdecl). So the fix you suggested is correct, I think. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-21 21:13 ` Jason Rumney @ 2005-05-22 21:21 ` Juanma Barranquero 2005-06-03 12:08 ` Juanma Barranquero 0 siblings, 1 reply; 37+ messages in thread From: Juanma Barranquero @ 2005-05-22 21:21 UTC (permalink / raw) > Studying the headers, png and jpeg explicitly use _cdecl (or __cdecl > depending on compiler), while xpm and gif specify nothing (so will > default to _cdecl). Yeah. And they indeed do use "ret" and not "ret n". > So the fix you suggested is correct, I think. Cool. So, for the first time, the image support on MSVC builds is as stable as on MinGW builds. -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-22 21:21 ` Juanma Barranquero @ 2005-06-03 12:08 ` Juanma Barranquero 2005-06-03 13:06 ` David Kastrup 0 siblings, 1 reply; 37+ messages in thread From: Juanma Barranquero @ 2005-06-03 12:08 UTC (permalink / raw) > Cool. So, for the first time, the image support on MSVC builds is as > stable as on MinGW builds. Well, I spoke too soon. I have another problem with image support on MSVC builds. It is, AFAICS, unrelated to the previous problem with the calling convention of the libraries. Doing (insert-image-file "c:/image/test/myimage.png") works, at works C-o from dired. However, either one of: (insert-image (create-image "c:/image/test/myimage.png")) (put-image (create-image "c:/image/test/myimage.png") 0) crashes. In other words: png_load, and jpeg_load, die an horrible death from: (image :type xxx :file "...") but they have no trouble with (image :type xxx :data "...") png_load, for example, dies at the call to fn_png_read_info (png_ptr, info_ptr); on png_load, while jpeg_load dies at: fn_jpeg_read_header (&cinfo, 1); Any clues about what can be happening? -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-06-03 12:08 ` Juanma Barranquero @ 2005-06-03 13:06 ` David Kastrup 2005-06-03 15:28 ` jasonr 2005-06-05 0:44 ` Juanma Barranquero 0 siblings, 2 replies; 37+ messages in thread From: David Kastrup @ 2005-06-03 13:06 UTC (permalink / raw) Cc: emacs-devel Juanma Barranquero <lekktu@gmail.com> writes: >> Cool. So, for the first time, the image support on MSVC builds is as >> stable as on MinGW builds. > > Well, I spoke too soon. > > I have another problem with image support on MSVC builds. It is, > AFAICS, unrelated to the previous problem with the calling convention > of the libraries. > > Doing > > (insert-image-file "c:/image/test/myimage.png") > > works, at works C-o from dired. However, either one of: > > (insert-image (create-image "c:/image/test/myimage.png")) > (put-image (create-image "c:/image/test/myimage.png") 0) > > crashes. In other words: png_load, and jpeg_load, die an horrible death from: > > (image :type xxx :file "...") > > but they have no trouble with > > (image :type xxx :data "...") > > png_load, for example, dies at the call to > > fn_png_read_info (png_ptr, info_ptr); > > on png_load, while jpeg_load dies at: > > fn_jpeg_read_header (&cinfo, 1); > > Any clues about what can be happening? The files are not opened in binary mode or something like that? -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-06-03 13:06 ` David Kastrup @ 2005-06-03 15:28 ` jasonr 2005-06-03 17:23 ` Stefan Monnier 2005-06-05 0:44 ` Juanma Barranquero 1 sibling, 1 reply; 37+ messages in thread From: jasonr @ 2005-06-03 15:28 UTC (permalink / raw) Cc: Juanma Barranquero, emacs-devel Quoting David Kastrup <dak@gnu.org>: > The files are not opened in binary mode or something like that? Its a possibility, but Emacs should be linked with binmode.obj, which makes the default mode binary. If the link with binmode.obj fails and default is text mode for some reason (as happened with certain versions of MingW for 21.4 and earlier), we usually get crashes loading .elc files either during startup or when opening custom buffers. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-06-03 15:28 ` jasonr @ 2005-06-03 17:23 ` Stefan Monnier 0 siblings, 0 replies; 37+ messages in thread From: Stefan Monnier @ 2005-06-03 17:23 UTC (permalink / raw) Cc: Juanma Barranquero, emacs-devel >> The files are not opened in binary mode or something like that? > Its a possibility, but Emacs should be linked with binmode.obj, which > makes the default mode binary. Also opening them in the wrong mode should only cause garbled images, not crashes, right? Stefan ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-06-03 13:06 ` David Kastrup 2005-06-03 15:28 ` jasonr @ 2005-06-05 0:44 ` Juanma Barranquero 2005-06-06 11:26 ` Juanma Barranquero 1 sibling, 1 reply; 37+ messages in thread From: Juanma Barranquero @ 2005-06-05 0:44 UTC (permalink / raw) On 6/3/05, David Kastrup <dak@gnu.org> wrote: > The files are not opened in binary mode or something like that? First thing I thought, but: /* Open the image file. */ fp = fopen (SDATA (file), "rb"); -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-06-05 0:44 ` Juanma Barranquero @ 2005-06-06 11:26 ` Juanma Barranquero 2005-06-14 2:02 ` Juanma Barranquero 0 siblings, 1 reply; 37+ messages in thread From: Juanma Barranquero @ 2005-06-06 11:26 UTC (permalink / raw) Funnier and funnier. I've been able to compile an MSVC debug release of libpng13; with it, both the MSVC and the GCC builds of Emacs crash. It happens inside msvcr71d.dll; (part of) the stack is: ntdll.dll!7c928fea() ntdll.dll!7c91104b() msvcr71d.dll!_lock_file(void * pf=0x77c2fce0) Line 236 C > msvcr71d.dll!fread(void * buffer=0x0082e268, unsigned int size=0x00000001, unsigned int count=0x00000004, _iobuf * stream=0x77c2fce0) Line 75 + 0x9 C libpng13d.dll!png_default_read_data(png_struct_def * png_ptr=0x20844540, unsigned char * data=0x0082e268, unsigned int length=0x00000004) Line 55 + 0x19 C libpng13d.dll!png_read_data(png_struct_def * png_ptr=0x20844540, unsigned char * data=0x0082e268, unsigned int length=0x00000004) Line 31 + 0x14 C libpng13d.dll!png_read_info(png_struct_def * png_ptr=0x20844540, png_info_struct * info_ptr=0x208483e0) Line 402 + 0xf C The crash is on the _lock_file() call inside the msvcr71 implementation of fread(); the relevant code is: void __cdecl _lock_file ( void *pf ) { /* * The way the FILE (pointed to by pf) is locked depends on whether * it is part of _iob[] or not */ if ( (pf >= (void *)_iob) && (pf <= (void *)(&_iob[_IOB_ENTRIES-1])) ) /* * FILE lies in _iob[] so the lock lies in _locktable[]. */ _lock( _STREAM_LOCKS + (int)((FILE *)pf - _iob) ); else /* * Not part of _iob[]. Therefore, *pf is a _FILEX and the * lock field of the struct is an initialized critical * section. */ EnterCriticalSection( &(((_FILEX *)pf)->lock) ); } The code decides that the FILE * is not part of _iob[] and calls EnterCriticalSection, which gets an access violation writing on 0x00000010. (I suppose I could get around the issue by writing read functions for libpng and jpeg and bypassing the standard code altogether; still, it'd be great to understand what's happening.) -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-06-06 11:26 ` Juanma Barranquero @ 2005-06-14 2:02 ` Juanma Barranquero 2005-06-14 7:32 ` David Kastrup 0 siblings, 1 reply; 37+ messages in thread From: Juanma Barranquero @ 2005-06-14 2:02 UTC (permalink / raw) > (I suppose I could get around the issue by writing read functions for > libpng and jpeg and bypassing the standard code altogether; And finally, that's what I've done. The following patch gets rid of png_init_io() and jpeg_stdio_src(), and instead define custom read functions to read image data from FILE *. With it, image support finally works as expected on MSVC and MinGW builds of Emacs on Windows. Highlights of the patch: - It has no ChangeLog entries :-) (I'll write them once the patch is accepted) - Deletion of png_init_io() and jpeg_stdio_src() is not conditional on HAVE_NTGUI; I've assumed it's best to have just one mechanism for Windows, Unix, GNU/Linux and MacOS platforms. I can reinstate the deleted functions and put the alternative into #ifdef's if deemed necessary. - I've changed a few names of the memory-reading data source functions for JPEG so it's clearer the paralelism between from-memory and from-file data source setups. - Most jpeg code I've added is heavily based on jdatasrc.c, from the JPEG library. I assume there's no legal trouble, as other jpeg code comes also from JPEG library examples. -- /L/e/k/t/u Index: src/image.c =================================================================== RCS file: /cvsroot/emacs/emacs/src/image.c,v retrieving revision 1.27 diff -u -2 -r1.27 image.c --- src/image.c 11 Jun 2005 16:24:36 -0000 1.27 +++ src/image.c 14 Jun 2005 01:46:10 -0000 @@ -5632,5 +5632,4 @@ DEF_IMGLIB_FN (png_destroy_read_struct); DEF_IMGLIB_FN (png_set_read_fn); -DEF_IMGLIB_FN (png_init_io); DEF_IMGLIB_FN (png_set_sig_bytes); DEF_IMGLIB_FN (png_read_info); @@ -5664,5 +5663,4 @@ LOAD_IMGLIB_FN (library, png_destroy_read_struct); LOAD_IMGLIB_FN (library, png_set_read_fn); - LOAD_IMGLIB_FN (library, png_init_io); LOAD_IMGLIB_FN (library, png_set_sig_bytes); LOAD_IMGLIB_FN (library, png_read_info); @@ -5690,5 +5688,4 @@ #define fn_png_destroy_read_struct png_destroy_read_struct #define fn_png_set_read_fn png_set_read_fn -#define fn_png_init_io png_init_io #define fn_png_set_sig_bytes png_set_sig_bytes #define fn_png_read_info png_read_info @@ -5763,4 +5760,21 @@ +/* Function set as reader function when reading PNG image from a file. + PNG_PTR is a pointer to the PNG control structure. Copy LENGTH + bytes from the input to DATA. */ + +static void +png_read_from_file (png_ptr, data, length) + png_structp png_ptr; + png_bytep data; + png_size_t length; +{ + FILE *fp = (FILE *) fn_png_get_io_ptr (png_ptr); + + if (fread (data, 1, length, fp) < length) + fn_png_error (png_ptr, "Read error"); +} + + /* Load PNG image IMG for use on frame F. Value is non-zero if successful. */ @@ -5896,5 +5910,5 @@ fn_png_set_read_fn (png_ptr, (void *) &tbr, png_read_from_memory); else - fn_png_init_io (png_ptr, fp); + fn_png_set_read_fn (png_ptr, (void *) fp, png_read_from_file); fn_png_set_sig_bytes (png_ptr, sizeof sig); @@ -6295,5 +6309,4 @@ DEF_IMGLIB_FN (jpeg_read_header); DEF_IMGLIB_FN (jpeg_read_scanlines); -DEF_IMGLIB_FN (jpeg_stdio_src); DEF_IMGLIB_FN (jpeg_std_error); DEF_IMGLIB_FN (jpeg_resync_to_restart); @@ -6311,5 +6324,4 @@ LOAD_IMGLIB_FN (library, jpeg_start_decompress); LOAD_IMGLIB_FN (library, jpeg_read_header); - LOAD_IMGLIB_FN (library, jpeg_stdio_src); LOAD_IMGLIB_FN (library, jpeg_CreateDecompress); LOAD_IMGLIB_FN (library, jpeg_destroy_decompress); @@ -6337,5 +6349,4 @@ #define fn_jpeg_read_header jpeg_read_header #define fn_jpeg_read_scanlines jpeg_read_scanlines -#define fn_jpeg_stdio_src jpeg_stdio_src #define fn_jpeg_std_error jpeg_std_error #define jpeg_resync_to_restart_wrapper jpeg_resync_to_restart @@ -6359,4 +6370,15 @@ +/* Method to terminate data source. Called by + jpeg_finish_decompress() after all data has been processed. + Common to memory and file source managers. */ + +static void +our_common_term_source (cinfo) + j_decompress_ptr cinfo; +{ +} + + /* Init source method for JPEG data source manager. Called by jpeg_read_header() before any data is actually read. See @@ -6364,5 +6386,5 @@ static void -our_init_source (cinfo) +our_memory_init_source (cinfo) j_decompress_ptr cinfo; { @@ -6375,5 +6397,5 @@ static boolean -our_fill_input_buffer (cinfo) +our_memory_fill_input_buffer (cinfo) j_decompress_ptr cinfo; { @@ -6395,5 +6417,5 @@ static void -our_skip_input_data (cinfo, num_bytes) +our_memory_skip_input_data (cinfo, num_bytes) j_decompress_ptr cinfo; long num_bytes; @@ -6412,14 +6434,4 @@ -/* Method to terminate data source. Called by - jpeg_finish_decompress() after all data has been processed. */ - -static void -our_term_source (cinfo) - j_decompress_ptr cinfo; -{ -} - - /* Set up the JPEG lib for reading an image from DATA which contains LEN bytes. CINFO is the decompression info structure created for @@ -6445,9 +6457,9 @@ src = (struct jpeg_source_mgr *) cinfo->src; - src->init_source = our_init_source; - src->fill_input_buffer = our_fill_input_buffer; - src->skip_input_data = our_skip_input_data; + src->init_source = our_memory_init_source; + src->fill_input_buffer = our_memory_fill_input_buffer; + src->skip_input_data = our_memory_skip_input_data; src->resync_to_restart = jpeg_resync_to_restart_wrapper; /* Use default method. */ - src->term_source = our_term_source; + src->term_source = our_common_term_source; src->bytes_in_buffer = len; src->next_input_byte = data; @@ -6455,4 +6467,136 @@ +/* JPEG data source manager setup for JPEG images read from a file. + Heavily based on jdatasrc.c from the JPEG library. */ + +struct my_jpeg_file_src { + struct jpeg_source_mgr pub; /* public fields */ + + FILE *infile; /* source stream */ + JOCTET *buffer; /* start of buffer */ + boolean start_of_file; /* have we gotten any data yet? */ +}; + +typedef struct my_jpeg_file_src *my_jpeg_file_ptr; + +#define JPEG_INPUT_BUF_SIZE 4096 /* choose an efficiently fread'able size */ + + +/* Init source method for JPEG data source manager. + Called by jpeg_read_header() before any data is actually read. */ + +static void +our_file_init_source (j_decompress_ptr cinfo) +{ + my_jpeg_file_ptr src = (my_jpeg_file_ptr) cinfo->src; + + /* We reset the empty-input-file flag for each image, + * but we don't clear the input buffer. + * This is correct behavior for reading a series of images from one source. + */ + src->start_of_file = 1; +} + + +/* Fill input buffer method for JPEG data source manager. + Called whenever more data is needed. */ + +static boolean +our_file_fill_input_buffer (j_decompress_ptr cinfo) +{ + my_jpeg_file_ptr src = (my_jpeg_file_ptr) cinfo->src; + size_t nbytes; + + nbytes = fread (src->buffer, 1, JPEG_INPUT_BUF_SIZE, src->infile); + + if (nbytes <= 0) { + if (src->start_of_file) /* Treat empty input file as fatal error */ + ERREXIT (cinfo, JERR_INPUT_EMPTY); + WARNMS (cinfo, JWRN_JPEG_EOF); + /* Insert a fake EOI marker */ + src->buffer[0] = (JOCTET) 0xFF; + src->buffer[1] = (JOCTET) JPEG_EOI; + nbytes = 2; + } + + src->pub.next_input_byte = src->buffer; + src->pub.bytes_in_buffer = nbytes; + src->start_of_file = 0; + + return 1; +} + + +/* Method to skip over NUM_BYTES bytes in the image data. + CINFO->src is the JPEG data source manager. */ + +static void +our_file_skip_input_data (j_decompress_ptr cinfo, long num_bytes) +{ + my_jpeg_file_ptr src = (my_jpeg_file_ptr) cinfo->src; + + /* Just a dumb implementation for now. Could use fseek() except + * it doesn't work on pipes. Not clear that being smart is worth + * any trouble anyway --- large skips are infrequent. + */ + if (num_bytes > 0) + { + while (num_bytes > (long) src->pub.bytes_in_buffer) + { + num_bytes -= (long) src->pub.bytes_in_buffer; + (void) our_file_fill_input_buffer (cinfo); + /* note we assume that fill_file_input_buffer will never return FALSE, + * so suspension need not be handled. + */ + } + src->pub.next_input_byte += (size_t) num_bytes; + src->pub.bytes_in_buffer -= (size_t) num_bytes; + } +} + + +/* Set up the JPEG lib for reading an image from a FILE * via + a customized function, thus avoiding FILE * conflicts between + different builds of the image library. CINFO is the + decompression info structure created for reading the image. */ + +static void +jpeg_file_src (cinfo, infile) + j_decompress_ptr cinfo; + FILE *infile; +{ + my_jpeg_file_ptr src; + + /* The source object and input buffer are made permanent so that a series + * of JPEG images can be read from the same file by calling jpeg_stdio_src + * only before the first one. (If we discarded the buffer at the end of + * one image, we'd likely lose the start of the next one.) + * This makes it unsafe to use this manager and a different source + * manager serially with the same JPEG object. Caveat programmer. + */ + if (cinfo->src == NULL) + { + /* first time for this JPEG object? */ + cinfo->src = (struct jpeg_source_mgr *) + (*cinfo->mem->alloc_small) ((j_common_ptr) cinfo, JPOOL_PERMANENT, + sizeof (struct my_jpeg_file_src)); + src = (my_jpeg_file_ptr) cinfo->src; + src->buffer = (JOCTET *) + (*cinfo->mem->alloc_small) ((j_common_ptr) cinfo, JPOOL_PERMANENT, + JPEG_INPUT_BUF_SIZE * sizeof (JOCTET)); + } + + src = (my_jpeg_file_ptr) cinfo->src; + src->pub.init_source = our_file_init_source; + src->pub.fill_input_buffer = our_file_fill_input_buffer; + src->pub.skip_input_data = our_file_skip_input_data; + src->pub.resync_to_restart = jpeg_resync_to_restart_wrapper; /* use default method */ + src->pub.term_source = our_common_term_source; + src->infile = infile; + src->pub.bytes_in_buffer = 0; /* forces fill_input_buffer on first read */ + src->pub.next_input_byte = NULL; /* until buffer loaded */ +} + + /* Load image IMG for use on frame F. Patterned after example.c from the JPEG lib. */ @@ -6538,5 +6682,5 @@ if (NILP (specified_data)) - fn_jpeg_stdio_src (&cinfo, (FILE *) fp); + jpeg_file_src (&cinfo, fp); else jpeg_memory_src (&cinfo, SDATA (specified_data), ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-06-14 2:02 ` Juanma Barranquero @ 2005-06-14 7:32 ` David Kastrup 2005-06-14 9:15 ` Juanma Barranquero 0 siblings, 1 reply; 37+ messages in thread From: David Kastrup @ 2005-06-14 7:32 UTC (permalink / raw) Cc: emacs-devel Juanma Barranquero <lekktu@gmail.com> writes: > - Most jpeg code I've added is heavily based on jdatasrc.c, from > the JPEG library. I assume there's no legal trouble, as other jpeg > code comes also from JPEG library examples. The examples are just that: examples. They are supposed to illustrate the most basic/natural use of the library and so don't contain copyrightable material unless somebody defined the interface very badly. You can claim copyright only on passages of sufficient creative value, and examples often are explicitly exempt (don't know whether this is the case here). So you can't judge the viability of taking core code (it sounds to me like that is what you did) by the viability of following example code. -- David Kastrup, Kriemhildstr. 15, 44793 Bochum ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-06-14 7:32 ` David Kastrup @ 2005-06-14 9:15 ` Juanma Barranquero 0 siblings, 0 replies; 37+ messages in thread From: Juanma Barranquero @ 2005-06-14 9:15 UTC (permalink / raw) Cc: emacs-devel On 6/14/05, David Kastrup <dak@gnu.org> wrote: > So you can't judge the viability of taking core code (it sounds to me > like that is what you did) by the viability of following example code. The code for the memory data source in image.c is already pretty similar. In any case, I found this bit from install.doc, which sums up the situation perfectly: 2. Microsoft C cannot pass file pointers between applications and DLLs. (See Microsoft Knowledge Base, PSS ID Number Q50336.) So jdatasrc.c and jdatadst.c don't work if you open a file in your application and then pass the pointer to the DLL. One workaround is to make jdatasrc.c/jdatadst.c part of your main application rather than part of the DLL. -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 15:46 ` Jason Rumney 2005-05-20 15:52 ` Eli Zaretskii @ 2005-05-20 16:31 ` Juanma Barranquero 1 sibling, 0 replies; 37+ messages in thread From: Juanma Barranquero @ 2005-05-20 16:31 UTC (permalink / raw) > Without looking at the disassembly, I'd guess that MinGW gcc and > non-optimised MSVC builds restore the stack pointer from a known > location regardless of the calling convention, which should always work. Optimized MSVC builds also restore the stack and frame pointers. But before that it tries to pop local registers saved on the stack. Bingo! My guess (but I've not compiled un-optimized to test it) is that non-optimized MSVC builds work because the compiler is reloading variables and/or being much less aggresive in caching values on registers. -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 14:44 ` Eli Zaretskii 2005-05-20 15:46 ` Jason Rumney @ 2005-05-20 16:29 ` Juanma Barranquero 1 sibling, 0 replies; 37+ messages in thread From: Juanma Barranquero @ 2005-05-20 16:29 UTC (permalink / raw) > It's the issue with both: the prototypes in the header files should be > consistent with how the functions were compiled, since those > prototypes are the only way for the compiler to know how the functions > were compiled. If it doesn't know that, it might produce wrong code. Yes, in general, for static linking (either with static libraries, or with stubs to dynamic ones). But in this case, MSVC is not compiling/liking statically against any function as defined in the header. There are no TIFF functions, as far as it knows, just a bunch of variables of type FARPROC that we're assigning and calling, so whether there's FAR CDECL TIFFOpen(...); or FARPROC TIFFOpen(...); on the header should be irrelevant. MSVC knows nothing about TIFFOpen; it just knows there's a variable fn_TIFFOpen which gets initialized with a call to GetProcAddress() and called afterwards. And it's the type of fn_TIFFOpen which counts. > That seems to be the problem. What I still don't get is how come the > same declaration that uses FARPROC works for the MinGW build? Can you > figure this out? Perhaps Jason's right and the bug is just not noticeable. Think of this: if lookup_image didn't use a temporary variable that gets assigned to a register and accidentally clobbered by tiff_load, we wouldn't see the problem with MSVC either. > This doesn't say anything about the calling conventions, just about > the inner workings of the functions. So I'm not surprised that both > versions have the same effect. Well, I hoped that the Win32 version would use the more common convention for DLL exports on Windows (that is, pascal). But it doesn't. -- /L/e/k/t/u ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: Problem with library images on Windows (again) 2005-05-20 10:16 ` Jason Rumney 2005-05-20 10:42 ` Juanma Barranquero @ 2005-05-20 12:19 ` Benjamin Riefenstahl 1 sibling, 0 replies; 37+ messages in thread From: Benjamin Riefenstahl @ 2005-05-20 12:19 UTC (permalink / raw) Cc: emacs-devel Hi Jason, Jason Rumney writes: > The version of TIFF headers I have uses extern "C" if compiling > under C++, so I doubt the library is using Pascal calling > conventions. Maybe Juanma's library is different than mine. 'extern "C"' doesn't mean what you think it means, at least not in most compilers. In theory 'extern "<Language>"' is the C++ equivalent of the propriatory '__cdecl', '__stdcall' and '__pascal' keywords known from the C compilers. But the exact behaviour of that C++ feature is implementation defined, especially for languages other than "C". In practice with MSC++ and GCC, '__stdcall' and 'extern "C"' are *both* needed (or at least it's highly recommended to use both) in C++ mode to actually get '__stdcall' calling conventions and name mangling. benny ^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2005-06-14 9:15 UTC | newest] Thread overview: 37+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2005-05-18 15:53 Problem with library images on Windows (again) Juanma Barranquero 2005-05-18 19:30 ` Andreas Schwab 2005-05-18 22:44 ` Juanma Barranquero 2005-05-19 6:58 ` Jason Rumney 2005-05-18 22:16 ` Jason Rumney 2005-05-18 22:52 ` Juanma Barranquero 2005-05-19 3:50 ` Eli Zaretskii 2005-05-19 8:13 ` Juanma Barranquero 2005-05-19 19:05 ` Eli Zaretskii 2005-05-19 19:31 ` Juanma Barranquero 2005-05-19 19:32 ` Juanma Barranquero 2005-05-20 7:20 ` Eli Zaretskii 2005-05-20 8:24 ` Kim F. Storm 2005-05-20 10:01 ` Eli Zaretskii 2005-05-20 10:16 ` Jason Rumney 2005-05-20 10:42 ` Juanma Barranquero 2005-05-20 14:44 ` Eli Zaretskii 2005-05-20 15:46 ` Jason Rumney 2005-05-20 15:52 ` Eli Zaretskii 2005-05-20 16:33 ` Juanma Barranquero 2005-05-20 17:57 ` Juanma Barranquero 2005-05-21 10:21 ` Jason Rumney 2005-05-21 10:33 ` Juanma Barranquero 2005-05-21 21:13 ` Jason Rumney 2005-05-22 21:21 ` Juanma Barranquero 2005-06-03 12:08 ` Juanma Barranquero 2005-06-03 13:06 ` David Kastrup 2005-06-03 15:28 ` jasonr 2005-06-03 17:23 ` Stefan Monnier 2005-06-05 0:44 ` Juanma Barranquero 2005-06-06 11:26 ` Juanma Barranquero 2005-06-14 2:02 ` Juanma Barranquero 2005-06-14 7:32 ` David Kastrup 2005-06-14 9:15 ` Juanma Barranquero 2005-05-20 16:31 ` Juanma Barranquero 2005-05-20 16:29 ` Juanma Barranquero 2005-05-20 12:19 ` Benjamin Riefenstahl
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).