[poppler] Compiling poppler with clang
Albert Astals Cid
aacid at kde.org
Mon Nov 19 13:44:13 PST 2012
El Dilluns, 19 de novembre de 2012, a les 08:16:59, Adam Reichold va escriure:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> 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
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.19 (GNU/Linux)
>
> iQEcBAEBAgAGBQJQqdzrAAoJEPSSjE3STU34RisH/iqlAtA62bwUMNG9Ym6iRQhk
> 7RXIqGED7gC7PNMp8gi/dGBH+ziRjVtQfag9oyq4L0QgqEkEHa4hH55WVSnaScik
> 8qZrRkh9w8XNcG7ROURRbWniKYM0cj8Ao24ps6nJr6GODAS82LpWycFUgz+W4v9J
> lA0jvkzhpJoS700L5rwUkj5xh+2X98Qy556yt3Hew1T7M1pCOAEWGc794k56/d4q
> je/zec2xL2oMGP8jrEr01GDqshg+z/+dAqrkg+yJ6M3CzpCvA3iB/kJEQZAMFT1g
> Jy6o7tn41WTk2FYlWmgPxna6bA+GvR4JT8dxrA47zOcmqTVoOtOfpa0o9+Wfvss=
> =4ghL
> -----END PGP SIGNATURE-----
More information about the poppler
mailing list