Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-22 Thread Joseph S. Myers
On Mon, 12 Aug 2013, Caroline Tice wrote: I have finished my first pass at the porting documentation. It is now available on the vtable verification feature web site: http://gcc.gnu.org/wiki/vtv Please provide the links there in a form that does not require JavaScript to read them (in

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-22 Thread Caroline Tice
I was not aware that the links required Javascript. Can anybody tell me how to fix them so that they don't? -- Caroline Tice cmt...@google.com On Thu, Aug 22, 2013 at 9:58 AM, Joseph S. Myers jos...@codesourcery.com wrote: On Mon, 12 Aug 2013, Caroline Tice wrote: I have finished my first

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-22 Thread Caroline Tice
I believe I have figured this out and fixed the problem. If you are still seeing the issue, please let me know. -- Caroline Tice cmt...@google.com On Thu, Aug 22, 2013 at 10:07 AM, Caroline Tice cmt...@google.com wrote: I was not aware that the links required Javascript. Can anybody tell me

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-12 Thread Caroline Tice
On Thu, Aug 8, 2013 at 9:34 AM, Caroline Tice cmt...@google.com wrote: On Thu, Aug 8, 2013 at 3:27 AM, Rainer Orth r...@cebitec.uni-bielefeld.de wrote: Caroline, As an aside, I had a very quick look at libvtv to determine what's required for a port to a non-Linux platform. It would be

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-08 Thread Rainer Orth
Caroline, your libgcc ChangeLog entries are all broken: they lack the initial * as can easily be seen in Emacs' Change Log Mode. Please fix. As an aside, I had a very quick look at libvtv to determine what's required for a port to a non-Linux platform. It would be good if the requirements

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-08 Thread Iain Sandoe
Hi Caroline, On 8 Aug 2013, at 11:27, Rainer Orth wrote: As an aside, I had a very quick look at libvtv to determine what's required for a port to a non-Linux platform. Likewise.. It would be good if the requirements (currently, ELF, dl_iterate_phdr, __fortify_fail, certainly several

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-08 Thread Ramana Radhakrishnan
On 08/06/13 22:39, Benjamin De Kosnik wrote: +# Filter out unsupported systems. +case ${target} in + x86_64-*-linux* | i?86-*-linux*) + VTV_SUPPORTED=yes + ;; + powerpc*-*-linux*) + ;; + sparc*-*-linux*) + ;; + arm*-*-linux*) + ;; What about powerpc, sparc and arm? Why are they

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-08 Thread Joseph S. Myers
On Thu, 8 Aug 2013, Ramana Radhakrishnan wrote: I spent some time this morning looking into what it would take to enable this for arm*-*-linux* today and the first thing that I noticed is that the testsuite is essentially driven by a shell script that caters to native testing on the only port

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-08 Thread Caroline Tice
On Thu, Aug 8, 2013 at 5:55 AM, Ramana Radhakrishnan ramra...@arm.com wrote: On 08/06/13 22:39, Benjamin De Kosnik wrote: +# Filter out unsupported systems. +case ${target} in + x86_64-*-linux* | i?86-*-linux*) + VTV_SUPPORTED=yes + ;; + powerpc*-*-linux*) + ;; + sparc*-*-linux*) +

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-08 Thread Caroline Tice
On Thu, Aug 8, 2013 at 3:27 AM, Rainer Orth r...@cebitec.uni-bielefeld.de wrote: Caroline, your libgcc ChangeLog entries are all broken: they lack the initial * as can easily be seen in Emacs' Change Log Mode. Please fix. Wow, I'm really sorry about that! I don't know how that happened.

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-08 Thread Ramana Radhakrishnan
arm*-*-linux* is broken currently for what looks like the same reasons as the powerpc port. Have you tried Benjamin Kosnik's patch? Does it fix the problem? I hadn't but that seems to have fixed the issue. Thanks. I spent some time this morning looking into what it would take to

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-08 Thread Caroline Tice
On Wed, Aug 7, 2013 at 6:03 PM, Joseph S. Myers jos...@codesourcery.com wrote: Looking at the patch as committed, there seems to be some confusion about the nature of the --enable-vtable-verify configure option. Yes, there is a bit. It's documented only for libstdc++. Now, I still consider

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-08 Thread Joseph S. Myers
On Thu, 8 Aug 2013, Caroline Tice wrote: Actually if you ever want to use the feature with your compiler you should build your compiler with --enable-vtable-verify. This will, as you noted, insert calls in libstdc++ to build the verification data structures and to verify the virtual calls

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-08 Thread Jason Merrill
On 08/08/2013 06:34 PM, Caroline Tice wrote: Actually if you ever want to use the feature with your compiler you should build your compiler with --enable-vtable-verify. This will, as you noted, insert calls in libstdc++ to build the verification data structures and to verify the virtual calls

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-08 Thread Caroline Tice
Which version gets used depends on their ordering in the link line. When -fvtable-verify=std or -fvtable-verify=preinit is used, the gcc driver inserts -lvtv very early into the link line (earlier than -lstdc++), so this happens automatically. -- Caroline cmt...@google.com On Thu, Aug 8, 2013

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-07 Thread Dominique Dhumieres
Revision 201555 breaks boostrap on x86_64-apple-darwin10: ... Checking multilib configuration for libvtv... make all-recursive Making all in testsuite /bin/sh: line 0: cd: testsuite: No such file or directory make[4]: *** [all-recursive] Error 1 make[3]: *** [all] Error 2 make[2]: ***

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-07 Thread Caroline Tice
libvtv was supposed to be automatically disabled for darwin; we are in the process of trying to figure out why this did not work as it was supposed to. In the meantime, if you add --disable-libvtv explicitly to your configure command, that should turn off the attempts to configure/build it. I'll

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-07 Thread Iain Sandoe
hi Caroline, A (very) quick look suggests that only *) results in a return of UNSUPPORTED from libvtv/configure.tgt Do you know if anyone is working on a Darwin port for this lib? thanks, Iain On 7 Aug 2013, at 17:57, Caroline Tice wrote: libvtv was supposed to be automatically disabled

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-07 Thread Caroline Tice
Hi Iain, Thanks for the pointer (I had noticed this but I appreciate the help!). I do not know of anyone working on a Darwin port for this at this time. -- Caroline Tice cmt...@google.com On Wed, Aug 7, 2013 at 10:31 AM, Iain Sandoe i...@codesourcery.com wrote: hi Caroline, A (very) quick

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-07 Thread David Edelsohn
Caroline, When libvtv was checked in, libvtv/autom4te.cache subdirectory was committed as well. I don't think that this was intended. - David

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-07 Thread Caroline Tice
+ gcc_patches list On Wed, Aug 7, 2013 at 2:56 PM, Caroline Tice cmt...@google.com wrote: No, it was not intended. How do I undo that? -- Caroline cmt...@google.com On Wed, Aug 7, 2013 at 2:54 PM, David Edelsohn dje@gmail.com wrote: Caroline, When libvtv was checked in,

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-07 Thread Paolo Carlini
.. I removed it. Thanks, Paolo.

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-07 Thread Caroline Tice
Thank you! -- Caroline On Wed, Aug 7, 2013 at 3:09 PM, Paolo Carlini paolo.carl...@oracle.com wrote: .. I removed it. Thanks, Paolo.

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-07 Thread Michael Meissner
On Wed, Aug 07, 2013 at 02:29:35PM +0200, Dominique Dhumieres wrote: Revision 201555 breaks boostrap on x86_64-apple-darwin10: ... Checking multilib configuration for libvtv... make all-recursive Making all in testsuite /bin/sh: line 0: cd: testsuite: No such file or directory make[4]:

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-07 Thread Joseph S. Myers
Looking at the patch as committed, there seems to be some confusion about the nature of the --enable-vtable-verify configure option. It's documented only for libstdc++. Now, I still consider the existence of separate documentation for libstdc++ configure options to be unfortunate - I think

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-06 Thread Ian Lance Taylor
On Tue, Aug 6, 2013 at 10:35 AM, Caroline Tice cmt...@google.com wrote: choose_tmpdir(), from libiberty is not mentioned in any .h file, so cannot (at the moment) be used here. Is it OK for me to add choose_tmpdir to libiberty.h? Well, maybe. But the approach looks a bit odd to me. Also

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-06 Thread Ian Lance Taylor
On Tue, Aug 6, 2013 at 11:43 AM, Caroline Tice cmt...@google.com wrote: On Tue, Aug 6, 2013 at 11:31 AM, Ian Lance Taylor i...@google.com wrote: The output to the file doesn't have any indication of what file is being compiled, so it will be ambiguous when run in parallel. You are mistaken.

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-06 Thread Ian Lance Taylor
On Tue, Aug 6, 2013 at 12:07 PM, Caroline Tice cmt...@google.com wrote: On Tue, Aug 6, 2013 at 12:02 PM, Ian Lance Taylor i...@google.com wrote: On Tue, Aug 6, 2013 at 11:43 AM, Caroline Tice cmt...@google.com wrote: On Tue, Aug 6, 2013 at 11:31 AM, Ian Lance Taylor i...@google.com

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-06 Thread Caroline Tice
I was talking with Diego, and he suggested the possibility of putting the log files in the same directory that the gcc dump files go, i.e. the one specified by dump_base_name. Do you think that would be acceptable? -- Caroline Tice cmt...@google.com On Tue, Aug 6, 2013 at 12:12 PM, Ian Lance

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-06 Thread Caroline Tice
Actually, I think that was dump_dir_name. -- Caroline On Tue, Aug 6, 2013 at 1:12 PM, Caroline Tice cmt...@google.com wrote: I was talking with Diego, and he suggested the possibility of putting the log files in the same directory that the gcc dump files go, i.e. the one specified by

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-06 Thread Ian Lance Taylor
On Tue, Aug 6, 2013 at 1:12 PM, Caroline Tice cmt...@google.com wrote: I was talking with Diego, and he suggested the possibility of putting the log files in the same directory that the gcc dump files go, i.e. the one specified by dump_base_name. Do you think that would be acceptable? Sure.

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-06 Thread Benjamin De Kosnik
+# Filter out unsupported systems. +case ${target} in + x86_64-*-linux* | i?86-*-linux*) + VTV_SUPPORTED=yes + ;; + powerpc*-*-linux*) + ;; + sparc*-*-linux*) + ;; + arm*-*-linux*) + ;; What about powerpc, sparc and arm? Why are they mentioned here if no actual

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-06 Thread Diego Novillo
On Tue, Aug 6, 2013 at 2:39 PM, Benjamin De Kosnik b...@redhat.com wrote: +# Filter out unsupported systems. +case ${target} in + x86_64-*-linux* | i?86-*-linux*) + VTV_SUPPORTED=yes + ;; + powerpc*-*-linux*) + ;; + sparc*-*-linux*) + ;; + arm*-*-linux*) + ;; What

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-06 Thread Caroline Tice
I have made all the requested changes. I am in the process of bootstrapping and running the regressions one last time. Assuming the bootstrapping regression tests pass, is this ok to commit? -- Caroline cmt...@google.com On Tue, Aug 6, 2013 at 2:45 PM, Diego Novillo dnovi...@google.com

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-06 Thread Diego Novillo
On Tue, Aug 6, 2013 at 2:50 PM, Caroline Tice cmt...@google.com wrote: I have made all the requested changes. I am in the process of bootstrapping and running the regressions one last time. Assuming the bootstrapping regression tests pass, is this ok to commit? Sure. Diego.

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-05 Thread Diego Novillo
This looks almost ready to commit. Some comments below: Once this is committed, you should write a blurb in GCC's home page describing the contribution. === --- libgcc/vtv_start_preinit.c (revision 0) +++

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-05 Thread David Malcolm
On Mon, 2013-08-05 at 15:24 -0700, Diego Novillo wrote: This looks almost ready to commit. Some comments below: [...] +/* Definition of this optimization pass. */ + +struct gimple_opt_pass pass_vtable_verify = +{ + { + GIMPLE_PASS, + vtable-verify, /* name

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-02 Thread Diego Novillo
On Thu, Aug 1, 2013 at 1:19 PM, Benjamin De Kosnik b...@redhat.com wrote: You'll need a GWP to do the merge (Maybe Diego, Jason, Richard Henderson?) and then add yourself to MAINTAINERS for libvtv. Eh, no. We need a GWP to approve the final patch, but Caroline can and should do the merge

[PATCH/Merge Request] Vtable Verification feature.

2013-08-01 Thread Caroline Tice
Quick Reminder; The vtable verification feature (controlled by a flag) is designed to detect, at run time, if/when the vtable pointer in a C++ object has been corrupted, before allowing virtual calls through that pointer. If pointer corruption is detected, execution of the program is halted. I

Re: [PATCH/Merge Request] Vtable Verification feature.

2013-08-01 Thread Benjamin De Kosnik
Nice to see! I have created (with some help) a git branch on gcc.gnu.org to contain the vtable verification feature work. This work is now well integrated with GCC trunk, and the sources are in a good state for future work. I believe all previous review comments have been addressed. The