[poppler] Compiling poppler with clang

Adam Reichold adamreichold at myopera.com
Tue Nov 13 22:28:21 PST 2012


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

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?

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&)"?

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

iQEcBAEBAgAGBQJQozoFAAoJEPSSjE3STU34WSgIAKq8CWceSQbavyFRyos0QsXS
cHTeh0Um9yfC4dIiaqa5ZwFOVH0OGkjWe02FtcygQGfNCgWeEHmod2ONFM3px3YK
3m4L8d3O/mxxtJ3bOq9u8XVFoZN2+JFElWL/ZKdoELGd+g8LnrZ2yWTqKo4ZRqMT
yw9yfkjMI3gqryD/aYTF9vGIKbw10zCBH80myuINQbja+SoNbZF1qmQqyWd1cp6v
qdyDHgXNSF4vlbrIa0E4ju/MMqtvVVwK42IHa8toP/h53CzP5DweZXR+ln5/lMoG
SWJwDoQ731RY1U25f5xaom4ZwmQ0KO9lhmeYd0cvsyFDhQqg828NkN2pzUTvIGQ=
=AleA
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: function_copy_constructor.patch
Type: text/x-patch
Size: 3025 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20121114/c11e20d0/attachment-0001.bin>


More information about the poppler mailing list