[poppler] Compiling poppler with clang
Adam Reichold
adamreichold at myopera.com
Thu Nov 22 12:17:51 PST 2012
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hello,
I am sorry, I seem to be a bit disorganized at the moment. My test was
using multi-threading which Poppler's master does not (yet) support
and that was the reason for the crashes. (I was also using this to
test Thomas' changes.)
So no problem here. It doesn't crash. With or without the last patch I
sent.
Regards, Adam.
Am 22.11.2012 20:30, schrieb Albert Astals Cid:
> El Dijous, 22 de novembre de 2012, a les 08:40:26, Adam Reichold va
> escriure: Hello,
>
> Somehow, I saw this one coming... :-) There were two calls to base
> constructors missing. Fixed and appended.
>
> But the linked document still crashes Poppler. And for me, it does
> so with my patch applied or not. Master itself seems to crash in
> Splash, for example at:
>
> 0 __memset_x86_64 /usr/lib/libc.so.6 0x7ffff5388a2a 1 Splash::clear
> Splash.cc:1929 0x7ffff5027693 2 SplashOutputDev::startPage
> SplashOutputDev.cc:1422 0x7ffff4f563cb 3 Gfx::Gfx Gfx.cc:564
> 0x7ffff4fa02e9 4 Page::createGfx Page.cc:486 0x7ffff4fd964b 5
> Page::displaySlice Page.cc:515 0x7ffff4fd9898 6
> Poppler::Page::renderToImage poppler-page.cc:264 0x7ffff7ba7220
>
> This is
>
> if (bitmap->alpha) { memset(bitmap->alpha, alpha, bitmap->width *
> bitmap->height); }
>
> in Splash::clear. I also encountered other crashes connected to
> the Splash bitmap data. I'd say we should figure out this problem
> before I continue to fool around with compiler warnings.
>
>> Interesting, the file works fine for me :S
>
>> Can you run valgrind on it?
>
>> Cheers, Albert
>
>
> Regards, Adam.
>
> Am 21.11.2012 23:27, schrieb Albert Astals Cid:
>>>> El Dimarts, 20 de novembre de 2012, a les 08:05:56, Adam
>>>> Reichold va escriure: Hello Albert,
>>>>
>>>> I am beginning to understand why one would not touch this, if
>>>> it isn't necessary. :-(
>>>>
>>>> I redid the changes and found that I missed to copy
>>>> "SampledFunction::nSamples". Sorry. Tested it against the PDF
>>>> file you linked.
>>>>
>>>>> Now crashes in
>>>>> http://www.it.piaggio.com/media/mp3hybdepita.pdf
>>>>>
>>>>> Cheers, Albert
>>>>
>>>> On a different but related topic: I tested the first patch
>>>> using the test data repository and "make check", but the
>>>> number of documents in this set seems limited. But be more
>>>> helpful (I am also thinking of Thomas' threading patches), is
>>>> they a way to get or automatically generate the collection of
>>>> PDF files covered by the bug tracker? (Or whatever else you
>>>> use for full regression testing.)
>>>>
>>>> Best regards, Adam.
>>>>
>>>> Am 19.11.2012 22:44, schrieb Albert Astals Cid:
>>>>>>> El Dilluns, 19 de novembre de 2012, a les 08:16:59,
>>>>>>> Adam Reichold va escriure: Hello Albert,
>>>>>>>
>>>>>>> I am sorry about that. It seems like my editor messed
>>>>>>> up with the spaces since it seems to apply using
>>>>>>> "--ignore-space-change". I tried to reformat the patch
>>>>>>> using said option, hopefully this helps. (It still has
>>>>>>> some whitespace warnings but no more errors.)
>>>>>>>
>>>>>>>> Can you have a look at
>>>>>>>> https://bugs.freedesktop.org/attachment.cgi?id=18330
>>>>>>>>
>>>>>>>> The dash line that divides the the green rectangular
>>>>>>>> area changes from grayish to black when applying your
>>>>>>>> patch.
>>>>>>>>
>>>>>>>> Cheers, Albert
>>>>>>>
>>>>>>> Regards, Adam.
>>>>>>>
>>>>>>> On 18.11.2012 23:01, Albert Astals Cid wrote:
>>>>>>>>>> El Dimecres, 14 de novembre de 2012, a les
>>>>>>>>>> 18:48:26, Adam Reichold va escriure: Hello
>>>>>>>>>> again,
>>>>>>>>>>
>>>>>>>>>> The attached patch should rework the function
>>>>>>>>>> copy constructors.
>>>>>>>>>>
>>>>>>>>>>> Doesn't seem to apply on current master, can
>>>>>>>>>>> you please rebase it and make sure it applies?
>>>>>>>>>>>
>>>>>>>>>>> Cheers, Albert
>>>>>>>>>>
>>>>>>>>>> Best regards, Adam.
>>>>>>>>>>
>>>>>>>>>> On 14.11.2012 12:58, Albert Astals Cid wrote:
>>>>>>>>>>>>> El Dimecres, 14 de novembre de 2012, a les
>>>>>>>>>>>>> 07:28:21, Adam Reichold va escriure: Hello
>>>>>>>>>>>>> Albert,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Before I try the other classes, could you
>>>>>>>>>>>>> have a look at the
>>>>>>>>>>>>>
>>>>>>>>>>>>> attached patch for SampledFunction and tell
>>>>>>>>>>>>> if this is what
>>>>>>>>>>>>>
>>>>>>>>>>>>> you are looking for?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Would something like memcpy(sampleSize,
>>>>>>>>>>>>> func->sampleSize, funcMaxInputs *
>>>>>>>>>>>>> sizeof(int)); be preferred to the plain
>>>>>>>>>>>>> for-loops?
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yes. it's what the other functions use,
>>>>>>>>>>>>>> right?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Regards, Adam.
>>>>>>>>>>>>>
>>>>>>>>>>>>> P.S.: By the way, is there a reason to use
>>>>>>>>>>>>> the signature Function(const Function*) for
>>>>>>>>>>>>> the "what would be a copy constructor"
>>>>>>>>>>>>> instead of "Function(const Function&)"?
>>>>>>>>>>>>>
>>>>>>>>>>>>>> No clue, ask the guy that wrote them
>>>>>>>>>>>>>> (hint, he's not around)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Cheers, Albert
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 13.11.2012 23:28, Albert Astals Cid
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>> El Dilluns, 27 d'agost de 2012, a
>>>>>>>>>>>>>>>> les 22:31:13, Albert Astals Cid
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> va escriure:
>>>>>>>>>>>>>>>>> El Dilluns, 27 d'agost de 2012, a
>>>>>>>>>>>>>>>>> les 08:29:40, Thomas Freitag va
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> escriure:
>>>>>>>>>>>>>>>>>> On 27.08.2012 00:56, Albert
>>>>>>>>>>>>>>>>>> Astals Cid
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>> El Diumenge, 26 d'agost de
>>>>>>>>>>>>>>>>>>> 2012, a les 15:48:37, He Liu
>>>>>>>>>>>>>>>>>>> va
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> escriure:
>>>>>>>>>>>>>>>>>>>>>> 5. vtable pointer will
>>>>>>>>>>>>>>>>>>>>>> be overwritten
>>>>>>>>>>>>>>>>>>>>>> Function.cc:422:10:
>>>>>>>>>>>>>>>>>>>>>> warning: destination for
>>>>>>>>>>>>>>>>>>>>>> this 'memcpy' call is a
>>>>>>>>>>>>>>>>>>>>>> pointer to dynamic class
>>>>>>>>>>>>>>>>>>>>>> 'SampledFunction'; vtable
>>>>>>>>>>>>>>>>>>>>>> pointer will be
>>>>>>>>>>>>>>>>>>>>>> overwritten
>>>>>>>>>>>>>>>>>>>>>> [-Wdynamic-class-memaccess]
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
memcpy(this, func,
>>>>>>>>>>>>>>>>>>>>>> sizeof(SampledFunction));
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
~~~~~~ ^
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Function.cc:422:10:
>>>>>>>>>>>>>>>>>>>>>> note: explicitly cast the
>>>>>>>>>>>>>>>>>>>>>> pointer to silence this
>>>>>>>>>>>>>>>>>>>>>> warning
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> At least categrory 5.
>>>>>>>>>>>>>>>>>>>>>> sound serious to me, I
>>>>>>>>>>>>>>>>>>>>>> would never have copied
>>>>>>>>>>>>>>>>>>>>>> instances of C++ objects
>>>>>>>>>>>>>>>>>>>>>> in that way, because it
>>>>>>>>>>>>>>>>>>>>>> depends on the compiler
>>>>>>>>>>>>>>>>>>>>>> and the class if this
>>>>>>>>>>>>>>>>>>>>>> causes problems on
>>>>>>>>>>>>>>>>>>>>>> runtime, s. i.e.
>>>>>>>>>>>>>>>>>>>>>> http://weseetips.com/tag/afx_zero_init_object/,
>
>>>>>>>>>>>>>>>>>>>>>>
> Note this is memset-ing to 0, not memcpy-ing a
>
>>>>>>>>>>>>>>>>>>>>> class to itself. To be
>>>>>>>>>>>>>>>>>>>>> honest i agree memcpy'in a
>>>>>>>>>>>>>>>>>>>>> SampledFunction to a
>>>>>>>>>>>>>>>>>>>>> SampledFunction is ugly,
>>>>>>>>>>>>>>>>>>>>> but i fail to see why it
>>>>>>>>>>>>>>>>>>>>> would not work.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> It works, at least with the
>>>>>>>>>>>>>>>>>> actual used compilers. But it
>>>>>>>>>>>>>>>>>> works only, because the allocated
>>>>>>>>>>>>>>>>>> members of SampledFunction are
>>>>>>>>>>>>>>>>>> overwriten after doing the
>>>>>>>>>>>>>>>>>> memcpy. And this behaviour makes
>>>>>>>>>>>>>>>>>> it just more ugly in my eyes.
>>>>>>>>>>>>>>>>>> And this is the same with the
>>>>>>>>>>>>>>>>>> other memcpy's in
>>>>>>>>>>>>>>>>>> ExponentialFunction,
>>>>>>>>>>>>>>>>>> StitchingFunction and
>>>>>>>>>>>>>>>>>> PostScriptFunction. And it will
>>>>>>>>>>>>>>>>>> work till such time as everybody
>>>>>>>>>>>>>>>>>> who changes the class will not
>>>>>>>>>>>>>>>>>> forget to do it in the same way.
>>>>>>>>>>>>>>>>>> So if You are not willing to
>>>>>>>>>>>>>>>>>> change it (or let somebody else
>>>>>>>>>>>>>>>>>> change it, I know, never change
>>>>>>>>>>>>>>>>>> running code), we could use the
>>>>>>>>>>>>>>>>>> hint: explicitly cast the pointer
>>>>>>>>>>>>>>>>>> to silence this warning.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I'm fine with someone fixing the
>>>>>>>>>>>>>>>>> code. Less warnings is always good,
>>>>>>>>>>>>>>>>> but for master, there's no need to
>>>>>>>>>>>>>>>>> introduce a pontential breaker just
>>>>>>>>>>>>>>>>> because clang complains.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> So at the end noone volunteered to
>>>>>>>>>>>>>>>> re-rewite those functions not to use
>>>>>>>>>>>>>>>> memcpy over classes with virtuals?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Cheers, Albert
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Cheers, Albert
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Cheers, Thomas
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Hi Albert,
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> :-)
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> A pointer of type
>>>>>>>>>>>>>>>>>>>> SampleFunction* could be
>>>>>>>>>>>>>>>>>>>> pointing to an instance of a
>>>>>>>>>>>>>>>>>>>> SampleFunction sub-class,
>>>>>>>>>>>>>>>>>>>> which has different vtable
>>>>>>>>>>>>>>>>>>>> contents.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> No, it could not,
>>>>>>>>>>>>>>>>>>> SampledFunction does not have
>>>>>>>>>>>>>>>>>>> any childs and the function
>>>>>>>>>>>>>>>>>>> doing that memcopy is private
>>>>>>>>>>>>>>>>>>> anyway.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> As a result, one could
>>>>>>>>>>>>>>>>>>>> construct a SampleFunction
>>>>>>>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>>>>>>> SampleFunction(SampleFunction
>>>>>>>>>>>>>>>>>>>> *) using a pointer to a
>>>>>>>>>>>>>>>>>>>> sub-class instance, and
>>>>>>>>>>>>>>>>>>>> overwrite the
>>>>>>>>>>>>>>>>>>>> SampleFunction's vtable
>>>>>>>>>>>>>>>>>>>> address with the sub-class's
>>>>>>>>>>>>>>>>>>>> vtable address.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> I am not sure if it will lead
>>>>>>>>>>>>>>>>>>>> to any bugs/vulnerabilities
>>>>>>>>>>>>>>>>>>>> in this case, but it is not
>>>>>>>>>>>>>>>>>>>> safe practice in general.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Sure, i never said it was. I'm
>>>>>>>>>>>>>>>>>>> just saying i don't see why it
>>>>>>>>>>>>>>>>>>> would not work in our case.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Since the vtable structure
>>>>>>>>>>>>>>>>>>>> depends on how the compiler
>>>>>>>>>>>>>>>>>>>> is implemented, memcpy or
>>>>>>>>>>>>>>>>>>>> memset on object pointers
>>>>>>>>>>>>>>>>>>>> will generally lead to
>>>>>>>>>>>>>>>>>>>> undefined behaviors.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I'm far from a compiler expert,
>>>>>>>>>>>>>>>>>>> but one would hope that for a
>>>>>>>>>>>>>>>>>>> given class the compiler stores
>>>>>>>>>>>>>>>>>>> always the "stuff" in the same
>>>>>>>>>>>>>>>>>>> order in memory, so again, i
>>>>>>>>>>>>>>>>>>> fail to see why this should
>>>>>>>>>>>>>>>>>>> fail in our case.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Albert
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>>>>>>>> _______________________________________________
>
>>>>>>>>>>>>>>>>>>>>
> poppler mailing list poppler at lists.freedesktop.org
>
>>>>>>>>>>>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>
>>>>
>>>>>>>>>>>>>>>>>>>>
_______________________________________________ poppler
>>>>
>>>>>>>>>>>>>>>>>>> mailing list
>>>>>>>>>>>>>>>>>>> poppler at lists.freedesktop.org
>>>>>>>>>>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>
>>>>>>>>>>>>>>>>>>>
> .
>
>>>>>>>>>>>>>>>>>> _______________________________________________
>>>>
>>>>
>>>>>>>>>>>>>>>>>>
poppler mailing list poppler at lists.freedesktop.org
>>>>
>>>>>>>>>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>
>>>>>>>>>>>>>>>>>>
> _______________________________________________ poppler
>
>>>>>>>>>>>>>>>>> mailing list
>>>>>>>>>>>>>>>>> poppler at lists.freedesktop.org
>>>>>>>>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>
>>>>>>>>>>>>>>>>>
> _______________________________________________ poppler
>
>>>>>>>>>>>>>>>> mailing list
>>>>>>>>>>>>>>>> poppler at lists.freedesktop.org
>>>>>>>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>
>>>>
>>>>>>>>>>>>>>>>
_______________________________________________ poppler
>>>>
>>>>>>>>>>>>> mailing list poppler at lists.freedesktop.org
>>>>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>
>>>>>>>>>>>>>
> _______________________________________________ poppler
>
>>>>>>>>>> mailing list poppler at lists.freedesktop.org
>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>>>>
>>>>>>>
>>>>>>>>>>
_______________________________________________ poppler
>>>>>>> mailing list poppler at lists.freedesktop.org
>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>
>>>> _______________________________________________ poppler
>>>> mailing list poppler at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>
> _______________________________________________ poppler mailing
> list poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
iQEcBAEBAgAGBQJQrohvAAoJEPSSjE3STU34QWwH/iG0X6cRNffn20LfIYh7Urfe
ti8imLAehUbnVatICtuLP9ptXLd7+j5kOhDVnLClcDma3ggay8YFrwRh5KrssRrD
b0K12r2fqSbwRHQjvn/LScn498S9AlDxbs12+eAyFk2Fw0/bj/cxWHjN/fu1CK8u
QgMKWyQ/BQyx6uXUlzKGqahgZWRCM9tsdLWZxXmm3KCXRE9AwrfGg18zhtAWmZ7M
KVXXRGALiIm3gflYBQ27LyDN4mo1+l24fid+4yJcbEl4CmBcxJoLQGoxwrjCWDK6
pdqjg6LRU7iLkHOczLtSgBzvk5dC2H3s3pvXSyN/ojqEJEOPIyLZCQHjZJoPhSU=
=6JSu
-----END PGP SIGNATURE-----
More information about the poppler
mailing list