[poppler] Compiling poppler with clang
Albert Astals Cid
aacid at kde.org
Sun Nov 25 15:42:13 PST 2012
El Dijous, 22 de novembre de 2012, a les 21:17:51, Adam Reichold va escriure:
> -----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.
Commited your patch to master.
Cheers,
Albert
>
> 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-----
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
More information about the poppler
mailing list