r230314 - Fix codegen for virtual methods that are (re-) exported from multiple modules.

Filipe Cabecinhas filcab at gmail.com
Tue Feb 24 16:03:29 PST 2015 

It broke our bots and Takumi's too.
http://bb.pgr.jp/builders/ninja-clang-i686-msc18-R/builds/139/steps/test-clang/logs/stdio
Our bot's log: http://reviews.llvm.org/P96 (<- MSVC 2013 Update 4 + cmake
3.1.3 + ninja 1.5.3)


  F

On Tue, Feb 24, 2015 at 1:30 PM, Reid Kleckner <rnk at google.com> wrote:

> This fails locally for me inside the Microsoft vftable builder:
> Assertion failed: OverridersMap.count(std::make_pair(MD, BaseOffset)) &&
> "Did not find overrider!", file ..\tools\clang\lib\AST\VTableBuilder.cpp,
> line 142
>
> We probably need to do the same change to the other vtable builder.
>
> On Tue, Feb 24, 2015 at 1:06 AM, Manuel Klimek <klimek at google.com> wrote:
>
>> Author: klimek
>> Date: Tue Feb 24 03:06:28 2015
>> New Revision: 230314
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=230314&view=rev
>> Log:
>> Fix codegen for virtual methods that are (re-) exported from multiple
>> modules.
>>
>> Fixes multiple crashes where a non-canonical decl would be used as key
>> in a lookup.
>>
>> Added:
>>     cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/
>>     cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/a.h
>>     cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/b.h
>>     cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/c.h
>>
>> cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap
>>     cfe/trunk/test/Modules/merge-vtable-codegen.cpp
>> Modified:
>>     cfe/trunk/lib/AST/VTableBuilder.cpp
>>
>> Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=230314&r1=230313&r2=230314&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
>> +++ cfe/trunk/lib/AST/VTableBuilder.cpp Tue Feb 24 03:06:28 2015
>> @@ -411,7 +411,8 @@ void FinalOverriders::dump(raw_ostream &
>>    for (const auto *MD : RD->methods()) {
>>      if (!MD->isVirtual())
>>        continue;
>> -
>> +    MD = MD->getCanonicalDecl();
>> +
>>      OverriderInfo Overrider = getOverrider(MD, Base.getBaseOffset());
>>
>>      Out << "  ";
>> @@ -695,6 +696,7 @@ void VCallAndVBaseOffsetBuilder::AddVCal
>>    for (const auto *MD : RD->methods()) {
>>      if (!MD->isVirtual())
>>        continue;
>> +    MD = MD->getCanonicalDecl();
>>
>>      CharUnits OffsetOffset = getCurrentOffsetOffset();
>>
>> @@ -1514,6 +1516,7 @@ void ItaniumVTableBuilder::AddMethods(
>>    for (const auto *MD : RD->methods()) {
>>      if (!MD->isVirtual())
>>        continue;
>> +    MD = MD->getCanonicalDecl();
>>
>>      // Get the final overrider.
>>      FinalOverriders::OverriderInfo Overrider =
>> @@ -2196,6 +2199,7 @@ void ItaniumVTableBuilder::dumpLayout(ra
>>      // We only want virtual member functions.
>>      if (!MD->isVirtual())
>>        continue;
>> +    MD = MD->getCanonicalDecl();
>>
>>      std::string MethodName =
>>
>>  PredefinedExpr::ComputeName(PredefinedExpr::PrettyFunctionNoVirtual,
>>
>> Added: cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/a.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/a.h?rev=230314&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/a.h (added)
>> +++ cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/a.h Tue Feb 24
>> 03:06:28 2015
>> @@ -0,0 +1,8 @@
>> +#ifndef A_H
>> +#define A_H
>> +
>> +struct A {
>> +  virtual void x();
>> +};
>> +
>> +#endif
>>
>> Added: cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/b.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/b.h?rev=230314&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/b.h (added)
>> +++ cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/b.h Tue Feb 24
>> 03:06:28 2015
>> @@ -0,0 +1,17 @@
>> +#ifndef B_H
>> +#define B_H
>> +
>> +#include "a.h"
>> +
>> +class B : virtual public A {
>> +  virtual void x() {}
>> +};
>> +
>> +void b(A* p) {
>> +  p->x();
>> +  // Instantiating a class that virtually inherits 'A'
>> +  // triggers calculation of the vtable offsets in 'A'.
>> +  B b;
>> +}
>> +
>> +#endif
>>
>> Added: cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/c.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/c.h?rev=230314&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/c.h (added)
>> +++ cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/c.h Tue Feb 24
>> 03:06:28 2015
>> @@ -0,0 +1,6 @@
>> +#ifndef C_H
>> +#define C_H
>> +
>> +#include "a.h"
>> +
>> +#endif
>>
>> Added:
>> cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap?rev=230314&view=auto
>>
>> ==============================================================================
>> ---
>> cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap
>> (added)
>> +++
>> cfe/trunk/test/Modules/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap
>> Tue Feb 24 03:06:28 2015
>> @@ -0,0 +1,11 @@
>> +module "a" {
>> +  textual header "a.h"
>> +}
>> +
>> +module "b" {
>> +  header "b.h"
>> +}
>> +
>> +module "c" {
>> +  header "c.h"
>> +}
>>
>> Added: cfe/trunk/test/Modules/merge-vtable-codegen.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-vtable-codegen.cpp?rev=230314&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/merge-vtable-codegen.cpp (added)
>> +++ cfe/trunk/test/Modules/merge-vtable-codegen.cpp Tue Feb 24 03:06:28
>> 2015
>> @@ -0,0 +1,24 @@
>> +// RUN: rm -rf %t
>> +
>> +// First, build two modules that both re-export the same header.
>> +// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=b -o
>> %t/b.pcm -fmodule-maps \
>> +// RUN:     -emit-module
>> %S/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap \
>> +// RUN:     -I %S/Inputs/merge-vtable-codegen
>> +// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-name=c -o
>> %t/c.pcm -fmodule-maps \
>> +// RUN:     -emit-module
>> %S/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap \
>> +// RUN:     -I %S/Inputs/merge-vtable-codegen
>> +
>> +// Use the two modules in a single compile.
>> +// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fmodule-file=%t/b.pcm
>> -fmodule-file=%t/c.pcm \
>> +// RUN:
>>  -fmodule-map-file=%S/Inputs/merge-vtable-codegen/merge-vtable-codegen.modulemap
>> \
>> +// RUN:     -emit-llvm -o %t/test.o %s
>> +
>> +// Note that order is important:
>> +// Module 'c' just reexports A, while module 'b' defines a method that
>> uses a
>> +// virtual method of A.
>> +#include "Inputs/merge-vtable-codegen/c.h"
>> +#include "Inputs/merge-vtable-codegen/b.h"
>> +
>> +void t() {
>> +  b(nullptr);
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150224/b065a331/attachment.html>

More information about the cfe-commits mailing list