<div dir="ltr">It broke our bots and Takumi's too.<div><a href="http://bb.pgr.jp/builders/ninja-clang-i686-msc18-R/builds/139/steps/test-clang/logs/stdio">http://bb.pgr.jp/builders/ninja-clang-i686-msc18-R/builds/139/steps/test-clang/logs/stdio</a><br></div><div>Our bot's log: <a href="http://reviews.llvm.org/P96">http://reviews.llvm.org/P96</a> (<- MSVC 2013 Update 4 + cmake 3.1.3 + ninja 1.5.3)</div><div><br></div></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature">  F<br></div></div>
<br><div class="gmail_quote">On Tue, Feb 24, 2015 at 1:30 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">This fails locally for me inside the Microsoft vftable builder:<div>Assertion failed: OverridersMap.count(std::make_pair(MD, BaseOffset)) && "Did not find overrider!", file ..\tools\clang\lib\AST\VTableBuilder.cpp, line 142</div><div><br></div><div>We probably need to do the same change to the other vtable builder.</div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 24, 2015 at 1:06 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: klimek<br>
Date: Tue Feb 24 03:06:28 2015<br>
New Revision: 230314<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=230314&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=230314&view=rev</a><br>
Log:<br>
Fix codegen for virtual methods that are (re-) exported from multiple modules.<br>
<br>
Fixes multiple crashes where a non-canonical decl would be used as key<br>
in a lookup.<br>
<br>
Added:<br>
    cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/<br>
    cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/a.h<br>
    cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/b.h<br>
    cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/c.h<br>
    cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap<br>
    cfe/trunk/test/Modules/merge-vtable-codegen.cpp<br>
Modified:<br>
    cfe/trunk/lib/AST/VTableBuilder.cpp<br>
<br>
Modified: cfe/trunk/lib/AST/VTableBuilder.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=230314&r1=230313&r2=230314&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=230314&r1=230313&r2=230314&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/AST/VTableBuilder.cpp (original)<br>
+++ cfe/trunk/lib/AST/VTableBuilder.cpp Tue Feb 24 03:06:28 2015<br>
@@ -411,7 +411,8 @@ void FinalOverriders::dump(raw_ostream &<br>
   for (const auto *MD : RD->methods()) {<br>
     if (!MD->isVirtual())<br>
       continue;<br>
-<br>
+    MD = MD->getCanonicalDecl();<br>
+<br>
     OverriderInfo Overrider = getOverrider(MD, Base.getBaseOffset());<br>
<br>
     Out << "  ";<br>
@@ -695,6 +696,7 @@ void VCallAndVBaseOffsetBuilder::AddVCal<br>
   for (const auto *MD : RD->methods()) {<br>
     if (!MD->isVirtual())<br>
       continue;<br>
+    MD = MD->getCanonicalDecl();<br>
<br>
     CharUnits OffsetOffset = getCurrentOffsetOffset();<br>
<br>
@@ -1514,6 +1516,7 @@ void ItaniumVTableBuilder::AddMethods(<br>
   for (const auto *MD : RD->methods()) {<br>
     if (!MD->isVirtual())<br>
       continue;<br>
+    MD = MD->getCanonicalDecl();<br>
<br>
     // Get the final overrider.<br>
     FinalOverriders::OverriderInfo Overrider =<br>
@@ -2196,6 +2199,7 @@ void ItaniumVTableBuilder::dumpLayout(ra<br>
     // We only want virtual member functions.<br>
     if (!MD->isVirtual())<br>
       continue;<br>
+    MD = MD->getCanonicalDecl();<br>
<br>
     std::string MethodName =<br>
       PredefinedExpr::ComputeName(PredefinedExpr::PrettyFunctionNoVirtual,<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/a.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/a.h?rev=230314&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/a.h?rev=230314&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/a.h (added)<br>
+++ cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/a.h Tue Feb 24 03:06:28 2015<br>
@@ -0,0 +1,8 @@<br>
+#ifndef A_H<br>
+#define A_H<br>
+<br>
+struct A {<br>
+  virtual void x();<br>
+};<br>
+<br>
+#endif<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/b.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/b.h?rev=230314&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/b.h?rev=230314&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/b.h (added)<br>
+++ cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/b.h Tue Feb 24 03:06:28 2015<br>
@@ -0,0 +1,17 @@<br>
+#ifndef B_H<br>
+#define B_H<br>
+<br>
+#include "a.h"<br>
+<br>
+class B : virtual public A {<br>
+  virtual void x() {}<br>
+};<br>
+<br>
+void b(A* p) {<br>
+  p->x();<br>
+  // Instantiating a class that virtually inherits 'A'<br>
+  // triggers calculation of the vtable offsets in 'A'.<br>
+  B b;<br>
+}<br>
+<br>
+#endif<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/c.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/c.h?rev=230314&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/c.h?rev=230314&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/c.h (added)<br>
+++ cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/c.h Tue Feb 24 03:06:28 2015<br>
@@ -0,0 +1,6 @@<br>
+#ifndef C_H<br>
+#define C_H<br>
+<br>
+#include "a.h"<br>
+<br>
+#endif<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap?rev=230314&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap?rev=230314&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap (added)<br>
+++ cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap Tue Feb 24 03:06:28 2015<br>
@@ -0,0 +1,11 @@<br>
+module "a" {<br>
+  textual header "a.h"<br>
+}<br>
+<br>
+module "b" {<br>
+  header "b.h"<br>
+}<br>
+<br>
+module "c" {<br>
+  header "c.h"<br>
+}<br>
<br>
Added: cfe/trunk/test/Modules/merge-vtable-codegen.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-vtable-codegen.cpp?rev=230314&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-vtable-codegen.cpp?rev=230314&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/merge-vtable-codegen.cpp (added)<br>
+++ cfe/trunk/test/Modules/merge-vtable-codegen.cpp Tue Feb 24 03:06:28 2015<br>
@@ -0,0 +1,24 @@<br>
+// RUN: rm -rf %t<br>
+<br>
+// First, build two modules that both re-export the same header.<br>
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=b -o %t/b.pcm -fmodule-maps \<br>
+// RUN:     -emit-module %S/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap \<br>
+// RUN:     -I %S/Inputs/merge-vtable-codegen<br>
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=c -o %t/c.pcm -fmodule-maps \<br>
+// RUN:     -emit-module %S/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap \<br>
+// RUN:     -I %S/Inputs/merge-vtable-codegen<br>
+<br>
+// Use the two modules in a single compile.<br>
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-file=%t/b.pcm -fmodule-file=%t/c.pcm \<br>
+// RUN:     -fmodule-map-file=%S/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap \<br>
+// RUN:     -emit-llvm -o %t/test.o %s<br>
+<br>
+// Note that order is important:<br>
+// Module 'c' just reexports A, while module 'b' defines a method that uses a<br>
+// virtual method of A.<br>
+#include "Inputs/merge-vtable-codegen/c.h"<br>
+#include "Inputs/merge-vtable-codegen/b.h"<br>
+<br>
+void t() {<br>
+  b(nullptr);<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</div></div><br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>