summaryrefslogtreecommitdiff
path: root/pdf/pdf_colour.c
Commit message (Collapse)AuthorAgeFilesLines
* Graphics library - cleanup properly on failure to push pdf14 deviceKen Sharp2023-05-031-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | OSS-fuzz bug #58423 The problem is reported as a use-after-free, what I see is a colour space persisting until long after the PDF interpreter has been freed, and being cleaned up by the end of job restore. Because the colour space was created by the PDF interpreter it has a custom callback to free associated objects. But by the time we call that callback the PDF interpreter has vanished. This happens because in gx_pattern_load() we try to push the pdf14 compositor (the pattern has transparency) which fails. Instead of cleaning up we were immediately returning, which was leaving the colour space counted up, which is why it was not counted down and freed before the interpreter exits. Fix that here by using a 'goto' the cleanup code instead of returning the error code immediately. Also, noted in passing, we don't need to set the callback in pdfi_create_DeviceRGB(), because that is done in pdfi_gs_setrgbcolor. Not only that, but there are circumstances under which we do not want to set the callback (if the space came from PostScript not created by the PDF interpreter) and that is catered for in pdfi_gs_setrgbcolor() whereas it wasn't in pdfi_create_DeviceRGB. So remove the callback assignment.
* GhostPDF - prevent buffer overrun when evaluating functionsKen Sharp2023-04-061-0/+10
| | | | | | | | | | | | | | | | OSS-fuzz bug #57745 The problem in the report is that the BlackGeneration function is a 1-in 3-out function. It is required to be a 1-in, 1-out function. The result was that the evaluation was writing 3 floats to a 1 float buffer. Check the parameters of the function to make sure it is of the correct size before trying to evaluate it. I also desk-checked all the other uses of functions; most were already checking the function parameters but I found two more cases which were not. Fix the /Separation and DeviceN tint transform so that we check the number of inputs and outputs to make sure they are correct.
* Update postal address in file headersChris Liddell2023-04-041-2/+2
|
* GhostPDF + pdfwrite - emit an Alternate for some ICCBased spaces.Ken Sharp2023-02-281-4/+339
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Bug #705865 "PDF Writer is dropping the /Alternate color space for ICC based color profile" This commit starts off with the bug referenced above. Prior to this commit the PDF interpreter never set the /Alternate space for an ICCBased colour space. For rendering there is no need since Ghostscript will always use the ICC profile, or will set the /Alternate instead (if there is an error in the ICC profile) or will use the number of components (/N) to set a Device space if there is a fault in the ICC profile and no /Alternate. However, this means pdfwrite never sees an Alternate space to write out for an ICCBased space. This should not be a problem since the /Alternate is optional for an ICCBased space and indeed the PDF 2.0 specification states "PDF readers should not use this colour space". The file for the bug has a /ICCBased space where the /Alternate is an Lab space. Obviously any PDF consumer should use the ICCBased space but it seems that Chrome, Firefox, possibly other browsers cannot handle ICCBased colour and so drop back to the Alternate. Surprisingly they can handle Lab and get the expected colour. Obviously if we drop the /Alternate then these consumers cannot use Lab and have a last ditch fallback to RGB (based on the number of components, and that *is* in the spec). But RGB != Lab and so the colours are incorrect. Ideally we would simply use the existing colour space code and attach the alternate space to the ICCBased space's 'base_space' member. That would write everything perfectly well. But we can't do that because when we are called from Ghostscript the ICC profile cache is in GC'ed memory. So we have to create the ICCBased colour space in GC'ed memory too. We have special hackery in the PDF interpreter colour code to do exactly that. Colour spaces call back to the PDF interpreter when freed (with more hackery for ICCBased spaces), but if we create colour spaces in non-GC (PDF interpreter) memory and attach them to the ICCBased space in GC'ed memory then they can outlive the PDF interpreter, leading to crashes. I did start down the road of making all colour spaces in GC-ed memory, but that rapidly spiralled out of control because names needed to be allocated in GC'ed memory, and functions and well, all kinds of things. Without that we got crashes, and it quickly became clear the only real way to make this work would be to allocate everything in GC'ed memory which we really don't want to do. So instead I added a new enumerated type member to the colour space, in that member, if the current colour space is ICCBased, we store the type of Alternate that was supplied (if any). We only support DeviceGray, DeviceRGB, DeviceCMYK, CalGray, CalRGB and Lab. I also added the relevant parameters to the 'params' union of the colour space. In the PDF interpreter; add code to spot the afore-mentioned Alternate spaces if present, and if we haven't been forced to fall back to using the Alternate (or /N) because the ICC profile is broken. When we spot one of those spaces, set the colour space ICC_Alternate_space member appropriately and for the paramaterised spaces gather the parameter values and store them. In the pdfwrite device; if we are writing out an ICCBased space, and it's ICC_Alternate_space member is not gs_ICC_Alternate_none, create a ColorSpace resource and call the relevant space-specific code to create a colour space array with a name and dictionary containing the required parameters. Attach the resulting object to the ICCBased colour space by inserting it into the array with a /Alternate key. This also meant I needed to alter the parameters passed internally to pdf_make_iccbased so that we pass in the original colour space instead of the alternate space (which is always NULL currently). There are also a couple of fixes; when finalising a colour space check that the colour space is a DeviceN space before checking if the device_n structure in the params union has a non-zero devn_process_space. The new members in the union meant we could get here and think we needed to free the devn_process_space, causing a crash. In the PDF interpreter; there's a little clause in the PDF specification which mentions a CalCMYK space. Apparently this was never properly specified and so should be treated as DeviceCMYK. The PDF interpreter now does so. Finally another obsrvation; the initial code wrote the /Alternate space as a named colour space, eg: 19 0 obj <</N 3 /Alternate /R18 /Length 1972>>stream .... Where R18 is defined in the Page's ColorSpace Resources as a named resource: <</R18 18 0 R/R17 17 0 R/R20 20 0 R/R22 22 0 R>> endobj But this does not work with Chrome (I didn't test Firefox). For this to work with Chrome we have to reference the object directly, which should not be required IMO. I believe this to be (another) bug in Chrome's PDF handling.
* GhostPDF - clear stack after colour operations inside type 3 'd1'Ken Sharp2022-12-211-0/+42
| | | | | | | | | | | | | | | | | | | | "z:\tests\pdf2.0\pdf2ftsRel2\fts_23_2310.pdf" If a Type 3 font has a CharProc which uses the 'd1' operator to declare that it does not set colour, but then goes on to set the colour space or colour, we raise an error (correctly). But I'd missed the cs, CS, sc, SC, scn and SCN operators. So firstly add code to ignore the colour space change for cs and CS. Secondly ignore the actual colour from sc, SC, scn and SCN. This is a little more difficult. Because we haven't set the colour space, we don't know how many components should be on the stack. So we just clear the stack wholesale. The PDF file is already broken, and the stack ought to be clear anyway. This was wrongly causing a 'garbage left on stack' error.
* GhostPDF log error when ICCBased /N value does not match profileKen Sharp2022-12-161-8/+2
| | | | | | | | Bug691941.pdf When this happens we do the same as the old interpreter and use the number of components from the ICC profile, but we were doing it silently. Log an error for reporting.
* GhostPDF - more 'emulate the old interpreter' warningsKen Sharp2022-12-121-4/+12
| | | | | | | | | | | | | | | d1_with_color_ops.pdf Add a warning when a colour operator is used inside a type 3 font CharProc which uses 'd1' (uncoloured). The spec says we should ignore the operation, which we do, but the old interpreter also raised a warning. DML_003-2010-1_8.pdf Add a warning when the BBox of a Form has zero width or height (and so doesn't draw anything because we clip to the BBox).
* GhostPDF - initialise a block of memoryKen Sharp2022-11-251-0/+1
| | | | | | | | | | | | | | | OSS-fuzz 53758 The file has an NChannel colour space which is broken, one of the ink names for the Process colourants is neither a name nor a string. That caused us to exit, but we hadn't initialised all the process_names in the structure (we allocate a block of char *) to hold them. When we then freed the colour space it tried to release the allocated names, which used uninitialised memory and so crashed. Memset the whole block of pointers to NULL before we start to avoid this.
* GhostPDF - cgeck base space for Indexed colour spacesKen Sharp2022-11-221-2/+6
| | | | | | | | | | OSS-fuzz 53619 The fuzzed file has a /Indexed space with a base space which is an array [/Pattern]. Pattern spaces (and Indexed spaces) are not valid as the base space for an Indexed space. Check the base space and exit with an error if it is not a valid space.
* GhostPDF - check for self-referencing named ColorSpacesKen Sharp2022-11-161-0/+6
| | | | | | | | | | | | | | | | | | | | OSS-fuzz 53393 The fuzzed file corrupts a named ColorSpace in the Resources dictionary of the page. The name should be /DefaultRGB with an indirect reference to the space, what we actually get is : /ColorSpace<</De////////faultRGB 3 0 R>> The odd characters are 0x0F bytes. So that defines a named ColorSpace /De as being the name <0x2f0x0F> and then defines the named space /<0x2F0x0F> as being /<0x2F0x0F>, ie it is self-referencing. We detect that for indirect objects, when dereferencing them, but there's no convenient central place to do that for self-referencing names, we just have to watch out for it when trying to get the object. Added a new error to cater for this case.
* GhostPDF - detect failed ICC colour spacesKen Sharp2022-10-201-0/+5
| | | | | | | | | | | | | | From OSS-fuzz 52552 - command line: -K1048576 -r200x200 -sBandListStorage=memory -dMaxBitmap=0 -dBufferSpace=450k -dMediaPosition=1 -dcupsColorSpace=1 -dSAFER -dNOPAUSE -dBATCH -dDOINTERPOLATE -dNOMEDIAATTRS /bugs/oss-fuzz/52552.pdf This exhibits as a floating-point exception in the image scaling filter caused by -DDOINTERPOLATE, but the actual problem is that the colour space has zero components, leading to a divide by zero error. The actual problem is when setting an ICC colour space, the PDF interpreter was missing an error condition (expected number of colour components being 0).
* GhostPDL + GS - do not make PS colour spaces into PDF onesKen Sharp2022-10-131-6/+43
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This was reported (out of Bugzilla) by Robin Watts, noticed in passing while debugging a different problem. Using the bug file (bears.pdf) from Bug #705975 and the following command line: -r1500 -sDEVICE=tiffsep -o /temp/out.tif /temp/bears.pdf if the output file out.tif cannot be written (read-only or held open by another application) then this caused an error. On a Memento build on Windows it causes a corrupted block on exit, on Linux it was giving a warning about attempting to free an unknown PDF object type. In all cases the error occurred during the PostScript end of job restore when there should be no PDF objects and the PDF interpreter context had been released. The problem was due to colour spaces, when we create a colour space in PDF we set a callback, called when a colour space is freed, which does extra work specific to the PDF interpreter. However we were ending up adding these callbacks to colour spaces allocated by the PostScript interpreter. This comes about because gs_setcolorspace() does not alter the current colour space if we attempt to switch to the same space. So in this case the current colour space in the PostScript interpreter was DeviceGray and the PDF interpreter then set DeviceGray, which did not result in a new colour space in the graphics state. But we went ahead and assigned the colour callbacks assuming that it had. When we finally freed that space we would then attempt to handle it as a PDF space, but the PDF context had long gone, resulting in an error. To fix this we now check the colour space after calling gs_setcolorspace to see if it has changed. If it has not we do not assign the PDF colour callbacks; either we have previously done so (if its a PDF space) or it is a PostScript space and we absolutely do not want to set the callbacks. Why does this not show up normally ? Because we do a showpage after every page, which does an initgraphics, which resets the PostScript colour space, and at this point the PDF context is still valid. In order to get this problem we currently need 'showpage' to return an error. But it is still wrong, much better to avoid the situation.
* GhostPDF - performance optimisation for pdfi_stream_to_bufferKen Sharp2022-09-191-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | The original version of pdfi_stream_to_buffer() always took two passes over the data; firstly to find the size of the uncompressed data (PDF files don't normally contain this information) then allocation of a memory buffer and re-reading the data into the buffer. Sometimes, however, we know how big we expect the data to be; examples include Indexed colour lookup tables, threshold arrays and both Mask and SMask image data. This commit modifies pdfi_stream_to_buffer() so that if the caller knows the expected size it can set the 'bufferlen' parameter on call. The routine will then skip the initial read to determine the size, allocate the buffer and read the data. It is possible that the file does not contain enough data to satisy the request. In that case (return code is ERRC and the initial bufferlen was not 0) we discard the buffer, reset the expected size to 0, and go back to the previous method of reading the data twice so that we can return the actual amount of data available. This does mean that for files which do not contain as much data as they should, we will read the data three times, and therefore be slower. But for conforming files we skip a read, gaining us performance. The file which inspired this is tests_private/comparefiles/Bug689195.pdf and that file gets approximately a 15% performance gain from this.
* GhostPDF - Only get the required number of Range values for a ColorSpaceKen Sharp2022-08-301-1/+1
| | | | | | | | | Bug #705828 "stack-overflow in /targets/ghostscript/foo/./pdf/pdf_colour.c:2236 pdfi_create_colorspace_by_array" The code was reading all the entries in a Range array into a stack array. But we hadn't validated the maximum number in the array so we could overrun. Limit the copy to the maximum number required by the 'N' value (which is validated).
* GhostPDF - Fix memory with typecheck on /Separation Alternate spaceKen Sharp2022-07-291-0/+1
| | | | | | | | OSS-fuzz #44264 with Memento If the Alternate colour space for a /Separation space was neither a name nor an array then we return an error, but we didn't countdown the reference to the object leading to a memory leak.
* GhostPDF - fix memory leak with device spaces used as Default*Ken Sharp2022-07-261-0/+3
| | | | | | Test file bugzilla_pdf/0693548_09209_Book4_edit.pdf uses DeviceGray as a DefaultGray space. We weren't adding the colour callback on free to the Device spaces, which meant that the attached name would leak.
* GhostPDF - don't reuse stack referencesKen Sharp2022-06-221-10/+30
| | | | | | | | | | | | | OSS-fuzz bug 48167 demonstrated that there are still places where we reference an object on the stack, but do not count it up and expect it to remain valid. This is not a safe assumption. Triggering a repair can clear the stack, and in the case of the bug, a code error can lead to the stack object being removed. This commit goes through all the places that we seem to make that assumption and reworks them to instead take an explicit reference.
* PDFI: Use helper functions for converting pdf_obj -> numsRobin Watts2022-05-051-136/+36
| | | | | | | | | This includes functions to pop values off the stack, convert to the appropriate types etc, that encapsulate error handling etc. Use these throughout the code. This drastically simplifies lots of code, and avoids needless error-prone repetition. It also means we can add debug testing/warnings in many fewer places.
* pdfi: Introduce pdfi_type_of.Robin Watts2022-05-051-142/+174
| | | | | All code now reads pdf obj types using pdfi_type_of rather than directly accessing obj->type. This will help us in the next optimisation step.
* GhostPDF - fix OSS-fuzz #46252Ken Sharp2022-04-021-0/+4
| | | | | | | | If the result of looking up an ColorSpace object is a name, but the name has a length of 0 (ie it was '/') then don't try and find that name, that isn't legal. Instead raise an error.
* OSS-fuzz #45508 - fix error path reference countingKen Sharp2022-03-131-0/+1
| | | | | | | | | | | The reference counts are correct, but after we've discarded the reference held by pcs_alt (the alternate space for a /Separation space) we hadn't set the pointer to NULL, which meant a subsequent error would decrement pcs_alt again, leading to us freeing the colour space while the ICC profile cache still had a reference to it. Fix by simply setting pcs_alt to NULL in order to properly discard the reference.
* OSS-fuzz 45320 - illegal colour in CharProc before d0/d1Ken Sharp2022-03-071-4/+4
| | | | | | | | | | | | | | | | | | | | | | The file has a (fuzzed) CharProc which does an 'RG' operation before executing either d0 or d1, which is illegal according to the spec. In addition, because of the gstate hackery we are required to do to deal with the possibility of setcachedevice potentially pushing a new device and the 'helpfulness' of setcachedevice, this messes up the reference counting of the colour space, because it decrements the stroke colour space reference. See the comments in pdfi_d0 in pdf_font.c. To deal with this, we extend the existing code to check (and ignore, as per the spec) any colour space changes in the course of a CharProc which executes a d1. The 'd1' flag has changed into an enumerated type and we now start a CharProc by setting the flag to 'none' to indicate that we haven't encountered any d0 or d1. If we get any colour space or colour changes before we get a d0 or d1, then we ignore them. If we get a colour or space change, and we are in a d1, we also ignore them. This resolves the problem, though it is possible there are other kinds of graphics state changes we need to guard similarly.
* Add gsicc_cache.h to includes...Chris Liddell2022-03-031-0/+3
| | | | ... to get function prototype for gsicc_profiles_equal()
* Eliminate pdf_overprint_control_t in favour of gs_overprint_control_tChris Liddell2022-03-031-1/+1
| | | | Fixes compiler warning comparing different enum types.
* Overprint simulation and OutputIntent interactionMichael Vrhel2022-03-021-20/+35
| | | | | | | | | | | | | | | | | In cases where -dOverprint=/simulate and -dUsePDFX3Profile are used, if the page has overprint and no transparency push the pdf14 device and render the page to a transparency buffer to the Output intent color space. If the target device has a different ICC profile than the output intent (specified with something like -sOutputICCProfile="Fogra39.icc") then convert the output intent rendered buffer to the target device color space. Also make sure that the tiff device do not use the wrong profile in the header. Additionally, a problem was found where the pdf14 device was setting up color conversions using the perceptual rendering intent. Those transforms should be using the colorimetric rendering intent.
* Coverity IDs 374941, 375552, 375572 (redux)Ken Sharp2022-02-181-4/+5
| | | | | There were obviously duplicate cases of these which I missed because I find the Coverity web interface hard to use. Fixed them here.
* OSS-fuzz #44781 - Don't try to set colour space name if creation failsKen Sharp2022-02-181-4/+6
| | | | | | | | | Says it all really. We store the colour space name for named spaces, so we can spot attempts to change colour space using the existing current space. But if we fail to create/set the space, then it doesn't exist, so we can't apply the name to it.
* Coverity IDs 374941, 375491, 375506, 375529, 375552, 375572Ken Sharp2022-02-171-7/+16
| | | | | | | | Set the client_color 'pattern' member to 0 before calling gs_setcolor. Make sure we can't try to call gs_setcolor without initialising at least one paint value, by checking the number of components in the space is at least 1.
* pdfi: add option to not replace existing entries in dictionariesChris Liddell2022-02-161-2/+2
| | | | | | | | | | In certain cases, we want to ignore duplicate keys being written to a dictionary, so add a parameter to pdfi_dict_put_obj() telling it to behave thus. It saves the expense of doing a pdfi_dict_known_by_key() first. And take advantage of it in the Type 1 font code.
* Coverity ID #375143 - remove superfluous checkKen Sharp2022-02-141-1/+1
| | | | | | | ICC_obj cannot be NULL here Coverity ID #375142 - return on error in function stream For errors parsing a PDF function, we just return
* GhostPDF - Complete the ICC cache performance fixKen Sharp2022-02-111-20/+15
| | | | | | | Due to a slip-up with Git, commit 18c791d6c6945bc6d42f2841d6e5e4d2ad6ab171 only included part of the actual performance fix. The remainder is included here.
* GhostPDF - Use the ICC cache for ICCBased spacesKen Sharp2022-02-111-0/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is a performance enhancement, when we are asked to create a new ICCBased colour space, we now probe the ICC cache, using the object number of the ICC colour space as a key, to see if it has previously been created and cached. If we find the colour has already been cached, then use the cached copy instead of reading the ICC profile. This massively improves performance for file which do the equivalent of: q /CS1 cs ... Q q /CS1 cs ... Q We already have a check for named colour spaces which are the same as the current space, but the use of gsave/grestore was defeating that. The QL ATS 1.7 test file CDX3C1DC_Dist.pdf runs about 30 times faster with this change. Other files are also affected. This also fixes (I have no idea why) eci_altona-test-suite-v2_technical2_x4.pdf with the psdcmyk16 device. Thanks to Michael for the vital clue about the ICC override when we are processing a soft mask.
* oss-fuzz 44018: More Type 1 pattern cleanup on error.Chris Liddell2022-01-251-0/+1
| | | | | | | | Initialise pattern instance pointer, for safetly. Don't leave a pointer in place to a pattern instance freed on error. On error, free the pdfi pattern context.
* Don't countdown a non-existent patternChris Liddell2022-01-241-5/+6
| | | | | | | | | One error case where we're handling a pattern, but the error occurs before the pattern object is created. In that case, we shouldn't attempt to decrement the reference count of the local pattern reference. Noticed investigating oss-fuzz issue: 43991 but the error location doesn't match what oss-fuzz reports, so I'm not sure it's the same probem.
* Cleanup local pattern reference on errorChris Liddell2022-01-211-1/+1
| | | | | | | pdfi_setcolorN() would skip cleaning up the local reference to a pattern if an error occurred, causing memory to leak. Noticed when investigating oss-fuzz issue: 43904
* OSS-fuzz #43354 - Validate Default* colour spacesKen Sharp2022-01-101-1/+169
| | | | | | | | | | | We were not checking that a DefaultGray, DefaultRGB or DefaultCMYK space was not one of Lab, Pattern or Indexed nor that it had the correct number of components. This commit checks that, moves the loading of Default* colour spaces into pdf_colour.c instead of doing almost exactly the same complex task in two different places (neither of them the colour code) and if the space is illegal, raies a warning.
* OSS-fuzz #42973 - correct a countodownKen Sharp2022-01-081-3/+5
| | | | | | On error we were counting down an object, which would later be counted down again leading eventually to an attempt to use a stale cache entry and a seg fault.
* OSS-fuzz #42460 - Ensure valid N parameter for ICCBased spcesKen Sharp2021-12-281-0/+4
| | | | | N is specified to be 1, 3 or 4, it if was more than 4 we would later overrun a fixed size array holding the range values.
* OSS-fuzz #43041 - Check dereferenced nameKen Sharp2021-12-281-0/+6
| | | | | | When creating a colour space by name, make sure that the returned value from dereferencing a non-standard colour space name is not the same name we started with (a circular reference).
* OSS-fuzz #42698 - Error if no Vertices specified in PolyLineKen Sharp2021-12-201-5/+5
| | | | | | | | The PolyLine annotation wasn't checking the return from pdfi_dict_knownget_type() to see if the Key was not present. Reviewed all the occurences of pdfi_dict_knonget_* to try and catch any more of these, as there have been a couple.
* OSS-fuzz #42453Ken Sharp2021-12-181-0/+6
| | | | | | | | | | | The fuzzed file ended up trying to deal with a broken token, and in the process treated it as a sc instead of an scn. This caused us to get the number of components for a Pattern space, which is -1. That caused a seg fault. Fixed by checking the returned number of components and if it is -1 (a Pattern colour space) then throw an error for operators that don't support Pattern spaces.
* oss-fuzz 42370: Handle /Seperation space with no stream dictChris Liddell2021-12-161-2/+5
| | | | | | | | In this case we have a color space with a corrupt name, so it is not recognised as ICCBased, and drops into the /Separation case, but the stream dict is NULL, so we try to deference a dictionary pointer that is NULL. Just add a check to ensure that doesn't happen.
* Fix one memory leak with 0693548_09210_Book4_edit_final.pdfChris Liddell2021-12-021-4/+2
| | | | | | | | | When setting the DefaultGray or DefaultCMYK color spaces we set the space's interpreter_data to the PDF_NAME object of the color space and reference count the name. Unfortunately, we weren't setting the callback function pointer to deal with the interpreter_data when the color space was freed. Setting that callback fixes that memory leaks.
* GhostPDF - fix memory leak with colour spacesKen Sharp2021-11-231-3/+4
| | | | | | | | | | | | | If, while setting a colour space by name, we detect that the current space is already the required space, then we short-circuit the setting of the space. But we were doing that after retrieving the object representing the named colour space, and not counting down the object leading to a memory leak. Instead, we can check the space early and avoid creating the object at all.
* Coverity IDs 372275, 373352 and 373353Ken Sharp2021-10-021-1/+2
| | | | | | | | | | | Arising from the work to detect switching to the current colour space. In pdf_color.c, don't try and set the colour callback in a colour space if we failed to create the colour space. In zpdfops.c, move the freeing of the profile cache into the if clause that checks if pdfctx was allocated. This solves both 373352 and 373353. Also change the if(ctx) and if(pdfctx) to explicit checks against NULL.
* GhostPDF - Fix ICC profile caching with colour spacesKen Sharp2021-09-291-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit c21f5f70f6ef15f3d1ec1975d5510e2c0dd959fd improved the performance in part by spotting attempts to change colour space to the same space as the current, and avoiding that. This led to seg faults with pdfi running under Ghostscript... To detect duplicate spaces, we added the PDF name object of the space to the 'interpreter data' stored in the colour space. We then checked the current name against the name of the desired space. This caused problems with ICC spaces, because these maintain a reference to the colour space. If the profile was stored in the PostScript profile cache (which it would be, because the profile cache is stored in the grapchis state, which we inherit from the PostScript interpreter) then it would outlive the PDF interpreter, and when we came to free the cache it would try to free the colour space, which would try to free the PDF name object, which had already been freed. To work around this we replaced the profile cache in the graphics state with the profile cache from the PDF interpreter. Now when we free the PDF profile cache, it frees the colour spaces, and the PDF names, while they are still valid. However, we then discovered that the garbage collector could reclaim the colour spaces. This happens because no GC-visible object declares it has a reference to the colour space. It doesn't matter that the colour space has been counted up, the GC frees it. Now the profile cache in pdfi is pointing to a colour space which has gone away.... To get round this, we create a brand-new profile cache, using GC'ed memory (well actually the interpreter allocator, which is GC'ed for Ghostscript). When we switch to pdfi we replace the graphics state pdfi is using with the one from the PostScript environment, and at the same time we swap out the profile cache being used by that graphics state, so it uses our new one. On return to PostScript we swap them back. This allows the lifetime of the profiles in the profile cache to be correct, because they are visible to the garbage collector (the PDF context is a GC-visible object and enumerates its member profile cache) When we finalize the PDF context we count down the profile cache which dereferences all its entries. This effectively frees the colour space (the memory may not be reclaimed yet) which calls the interpreter specific cleanup, which frees the (still valid at this point) PDF name object and sets the pointer to NULL. The net result is that we have the profiles visible to the garbage collector during their lifetime, but still have control over the cache, so we can empty it of the profiles created for pdfi when we close the PDF interpreter.
* Ghostpdf - silence a couple of compiler warningsKen Sharp2021-09-241-1/+1
| | | | | | | Include pdf_misc.h to rpvide a function prototype Change a debug print to use a 64-bit format specifier instead of a long for a 64-bit variable.
* GhostPDF - performance enhancement on colour spacesKen Sharp2021-09-231-2/+32
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit checks the action of setting the current colour space to a named colour space, to ensure that it is not already the current colour space. This is unfortunately complicated. Because of gsave and grestore, we must store some object along with the colour space which indicates the PDF object that it is associated with, so that we can check the current fill or stroke colour space against the requested space. Fortunately we already have provision for storing the interpreter context in the colour space, in order to deal with functions describing the tint transform to the alternate space. We take advantage of this to store the PDF interpreter 'name' object instead, as PDF objects store a pointer to the context anyway, so we can retrieve the context from there in order to free functions. However... Not all colour spaces are associated with a PDF name. For example the Alternate space of a Separation or DeviceN space, and device spaces. In this case we do actually need to store a pointer to the PDF interpreter context. But we need to be able to tell which one we have stored. The simplest way to deal with this is to turn the PDF interpreter context into a PDF interpreter object, of a sort. We won't really reference count it, or consult it's flags, but we can check the object type to see what kind of object it is, and use the context pointer without concerns. The remaining ugliness is storing a pointer to the current 'name' in the PDF interpreter context when we are asked to set a named space, just so that at the bottom level we can store it in the colour space (and count it up!). I'd prefer to do this by passing a parameter but that requires some replumbing that I don't really want to tackle just yet. The colour space testing is conservative, if there is any doubt that the current space is not the same as the requested space, we will build and set the requested space. The test file I've been using, page 5 from the file sent by Bruno Voisin on gs-devel, now runs 2.4 times faster, and is broadly the same speed as the old PDF interpreter at screen resolution. The remaining pages seem to be broadly in line as well.
* GhostPDF - warn on use of /DeviceN /AllKen Sharp2021-08-311-1/+16
| | | | | | | | | | | | | The /All in name is illegal in PDF (it is valid in PostScript). Acrobat allegedly seems to accept this; if the ink name array only has one entry and the entry is /All it seems to convert it to a /Sepaaration /All, which is equivalent. However sometimes (tests_private/comparefiles/Testform.v1.0.2.pdf) it throws an error instead. Possibly this is a change in behaviour with more recent versions of Acrobat. In any event, we now raise a warning.
* Coverity IDs 372258, 372262, 372275, 372291, 372328Ken Sharp2021-08-141-8/+11
| | | | | | | | | Don't attempt to set the colour callback if the colour space creation failed (x3, one each for Gray, RGB and CMYK) Don't attempt to set the colour space if the creation failed. remove dead code