r230314 - Fix codegen for virtual methods that are (re-) exported from multiple modules.
NAKAMURA Takumi
geek4civic at gmail.com
Tue Feb 24 16:22:03 PST 2015
Reverted in r230406.Manuel, could you reconfirm with targeting
(i686|x86_64)-win32?
2015-02-25 9:03 GMT+09:00 Filipe Cabecinhas <filcab at gmail.com>:
> 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
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list