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

Reid Kleckner rnk at google.com
Tue Feb 24 13:30:13 PST 2015


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


More information about the cfe-commits mailing list