<div dir="ltr">It should pass after r230445 and r230446, I will try to reland it.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 24, 2015 at 4:22 PM, NAKAMURA Takumi <span dir="ltr"><<a href="mailto:geek4civic@gmail.com" target="_blank">geek4civic@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Reverted in r230406.Manuel, could you reconfirm with targeting<br>
(i686|x86_64)-win32?<br>
<div class="HOEnZb"><div class="h5"><br>
2015-02-25 9:03 GMT+09:00 Filipe Cabecinhas <<a href="mailto:filcab@gmail.com">filcab@gmail.com</a>>:<br>
> It broke our bots and Takumi's too.<br>
> <a href="http://bb.pgr.jp/builders/ninja-clang-i686-msc18-R/builds/139/steps/test-clang/logs/stdio" target="_blank">http://bb.pgr.jp/builders/ninja-clang-i686-msc18-R/builds/139/steps/test-clang/logs/stdio</a><br>
> Our bot's log: <a href="http://reviews.llvm.org/P96" target="_blank">http://reviews.llvm.org/P96</a> (<- MSVC 2013 Update 4 + cmake<br>
> 3.1.3 + ninja 1.5.3)<br>
><br>
><br>
>   F<br>
><br>
> On Tue, Feb 24, 2015 at 1:30 PM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:<br>
>><br>
>> This fails locally for me inside the Microsoft vftable builder:<br>
>> Assertion failed: OverridersMap.count(std::make_pair(MD, BaseOffset)) &&<br>
>> "Did not find overrider!", file ..\tools\clang\lib\AST\VTableBuilder.cpp,<br>
>> line 142<br>
>><br>
>> We probably need to do the same change to the other vtable builder.<br>
>><br>
>> On Tue, Feb 24, 2015 at 1:06 AM, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br>
>>><br>
>>> 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<br>
>>> 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>
>>><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:<br>
>>> <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>
>>> ==============================================================================<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>
>>><br>
>>> PredefinedExpr::ComputeName(PredefinedExpr::PrettyFunctionNoVirtual,<br>
>>><br>
>>> Added: cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/a.h<br>
>>> URL:<br>
>>> <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>
>>> ==============================================================================<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<br>
>>> 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:<br>
>>> <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>
>>> ==============================================================================<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<br>
>>> 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:<br>
>>> <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>
>>> ==============================================================================<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<br>
>>> 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:<br>
>>> cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap<br>
>>> URL:<br>
>>> <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>
>>> ==============================================================================<br>
>>> ---<br>
>>> cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap<br>
>>> (added)<br>
>>> +++<br>
>>> cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap<br>
>>> 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:<br>
>>> <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>
>>> ==============================================================================<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<br>
>>> 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<br>
>>> %t/b.pcm -fmodule-maps \<br>
>>> +// RUN:     -emit-module<br>
>>> %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<br>
>>> %t/c.pcm -fmodule-maps \<br>
>>> +// RUN:     -emit-module<br>
>>> %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<br>
>>> -fmodule-file=%t/c.pcm \<br>
>>> +// RUN:<br>
>>> -fmodule-map-file=%S/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap<br>
>>> \<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<br>
>>> 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">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>
>><br>
>><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>
><br>
><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>
</div></div></blockquote></div><br></div>