Re: [PATCH, updated] Vtable pointer verification, runtime library changes (patch 3 of 3)

[Jonathan Wakely <jwakely.gcc@gmail.com>]

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 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)

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?

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.

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
s/functionsa/functions/

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
s/pointters/pointers/

+   libvtv_init.so is built from vtv_init.cc.  It is designed to hel[p
s/hel\[p/help/


+   __VLTVerifyVtablePoitner) with stub functions that do nothing.  If
s/Poitner/Pointer/

+   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
s/secttion/section/

+      /* TODO: Meed to revisit this code for dlopen. It most probably
+         is not unlocking the protected vtable vars after for a load

s/Meed/Need/
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 :-)