Page MenuHomePhabricator

Always issue vtables when generating coverage instrumentation
AbandonedPublic

Authored by FlameTop on Dec 5 2016, 6:00 AM.

Details

Reviewers
rsmith
Summary

The provided test shows a case where enabling coverage instrumentation causes a link error during building. Normally the all the base class items (vtable, ctors and dtors) would be removed in an optimized build. But when building with coverage instrumentation the ctors and dtors are always issued and access the vtable. However, the vtable remains with external linkage. As nobody ever instantiates the base class the vtable remains undefined and causes a link error.

This patch changes the behavior when deciding the vtable linkage when generating coverage instrumentation. vtables are always generated as local linkonce objects.

This is slightly wasteful of link time and target storage but the profile instrumentation is performed too early (as far as I can see) to not instrument ctors/dtors that will later be discarded.

Diff Detail

Event Timeline

FlameTop updated this revision to Diff 80259.Dec 5 2016, 6:00 AM
FlameTop retitled this revision from to Always issue vtables when generating coverage instrumentation.
FlameTop updated this object.
FlameTop added a reviewer: rsmith.
FlameTop added a subscriber: cfe-commits.
ahatanak added inline comments.
llvm/tools/clang/test/CodeGenCXX/vtable-coverage-gen.cpp
3

I'm not sure I understood the purpose of this test, but It looks like the vtable for Category is generated in the IR with linkonce_odr without your patch.

vsk added a subscriber: vsk.Dec 19 2016, 11:13 AM
vsk added inline comments.Dec 19 2016, 11:23 AM
llvm/tools/clang/test/CodeGenCXX/vtable-coverage-gen.cpp
3

Yes, I'm seeing the same thing and am also confused. I can reproduce the build failure without using any vtables. To me this suggests the problem could be elsewhere. Here is a minimal reproducer:

struct Base {
  static const Base *get();
};

struct Derived : public Base {};

const Base *Base::get() {
  static Derived d;
  return &d;
}

int main() { return 0; }
vsk added inline comments.Dec 19 2016, 11:27 AM
llvm/tools/clang/test/CodeGenCXX/vtable-coverage-gen.cpp
3

^ Please ignore my last comment, I made a mistake while trying to compile your reproducer. When I used a proper compile command, I could not reproduce the build failure on Darwin/MachO (-fprofile-instr-generate -fcoverage-mapping).

FlameTop abandoned this revision.Dec 20 2016, 8:11 AM

I must apologise to you all. The problem was reported in our out-of-tree version of the compiler and I must have hit finger problems when I confirmed it in tree. Repeating the test I can no longer reproduce the fault with the 3.9.0 public build. It must be some effect of a local change.

Again apologies.

Phil