[poppler] Compiling poppler with clang

Wed Nov 14 09:48:26 PST 2012

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello again,

The attached patch should rework the function copy constructors.

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
> 
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)

iQEcBAEBAgAGBQJQo9lqAAoJEPSSjE3STU34xZEIAL34RZqQK3gnFvZ7LVYLM6rf
lpSG1qn0P0G5h1qexVtx9gMxoaN4m2lKMS2N9eomNu3IwxgP5D7phwdSfJesCkDS
gd19EQ+oDtwKdeLdQMq+oM63mYzr6nyVUHlPSFtIO4Z1VBdZCMsowQnTsUe45UJ9
HWzqzzwiX91OTh7ILZqpQ4UtgmjZRIZmGxJVOqJcAK+qdVh7HiYmCbjGKnZ+YVdp
NtW2jWTTg6GL+rEMQS1WTR1oDgfIPLgQWAdULezS3fsOQQvthUMlQuZQMgeG5cVR
QGbQIjKTLjw/TNo+oQ+5r/lLfHynFYp3VNuAv/LCIHMeGZANpfKaQeTaao2ecMk=
=fieh
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: function_copy_constructor.patch
Type: text/x-patch
Size: 10893 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20121114/b5b7e24c/attachment.bin>