This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH, updated] Vtable pointer verification, runtime library changes (patch 3 of 3)
On Mon, Feb 25, 2013 at 1:14 PM, Jonathan Wakely <email@example.com> wrote:
> On 25 February 2013 19:52, Caroline Tice wrote:
>> I got too excited about being done and forgot to attach the patch. :-(
>> Sorry. Here it is.
> Some comments follow, mostly from reading the comments to understand
> what this patch does, it's a really interesting feature!
> The generated files (configure, */Makefile.in) don't need to be in the
> patch and their ChangeLog entry can be simply "Regenerated"
> Was configure regenerated of modified by hand? When regenerating it
> with Autoconf 2.64 I get a different output.
I modified the configure file by hand. I didn't realize it was
> I also get this warning when regenerating the configury bits:
> src/Makefile.am:80: variable `libvtv___la_LIBADD' is defined but no program or
> src/Makefile.am:80: library has `libvtv___la' as canonical name (possible typo)
I was trying to define them as empty (this was in the else clause,
where "--enable-vtable-verify=yes" was not specified when doing the
configure. I should probably remove libvtv___la_LIBADD from the else
> The copyright dates should be updated to 2012-2013.
> vtv_add_to_log in libstdc++-v3/libsupc++/vtv_utils.cc uses va_start
> but the matching call to va_end is missing.
> Am I right in thinking there's no danger of namespace pollution from
> vtv_map.h etc. because those headers will never be included unless
> explicitly requested by users?
That should be correct.
> I'm not sure what the rules are regarding flexible array members in
> C++ (as it's an extension) but it looks like insert_only_hash_map is a
> non-POD (formally, it has non-trivial initialization) but no
> constructor or destructor runs for it, so formally the object's
> lifetime never begins or ends, it is just a block of memory that gets
> allocated, some bytes are set, then the memory is deallocated again.
I will go back and look at this again. It's been a while since I
looked at that code.
> The comments in libstdc++-v3/libsupc++/vtv_init.cc have a typo:
> +/* This file contains all the definitions that go into the libvtv_init
> + library, which is part of the vtable verification feature. This
> + library should contain exactly two functionsa (__VLTunprotect and
I'll fix that.
> And in the comments in libstdc++-v3/libsupc++/vtv_rts.cc
> + The actual set of valid vtable pointers for a virtual class,
> Should that be "polymorphic class" instead of "virtual class"?
> Further on there are a few typos:
> + pointters for the class, so we wrote our own hashtable-based symbol
> + libvtv_init.so is built from vtv_init.cc. It is designed to hel[p
> + __VLTVerifyVtablePoitner) with stub functions that do nothing. If
> + initialize any of these statics with a runtime call (for ex:
> + sysconf.
> (Unclosed parenthesis)
> + the secttion offset and size, in conjunction with the data in INFO
> + /* TODO: Meed to revisit this code for dlopen. It most probably
> + is not unlocking the protected vtable vars after for a load
> s/after for a load/after a load/
> I see a few TODO comments in the code, I assume the plan is to address
> them eventually as time permits, rather than this being a code-drop
> that becomes abandonware :-)
Yes. They are things that we are planning on addressing but did not
seem worth holding up the patch for.