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

Reid Kleckner rnk at google.com
Tue Feb 24 18:19:02 PST 2015 

It should pass after r230445 and r230446, I will try to reland it.

On Tue, Feb 24, 2015 at 4:22 PM, NAKAMURA Takumi <geek4civic at gmail.com>
wrote:

> 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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150224/8988c32f/attachment.html>

More information about the cfe-commits mailing list