Skip to content

export classes with pure virtual destructors #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

andrurogerz
Copy link
Collaborator

Purpose

IDS should annotate classes that have pure virtual destructors with out-of-line implementations. I have observed a small number of these cases while annotating the LLVM codebase and found it was easy to identify and fix automatically with IDS.

Validation

  1. New test cases.
  2. Ran all tests on Linux and Windows.
  3. Manually ran on LLVM source and verified it found and fixed 3 instances.

@andrurogerz andrurogerz requested a review from compnerd May 7, 2025 23:42
// an out-of-line implementation. If a pure virtual destructor is found
// that has no body and is not defaulted, it must have an out-of-line
// implementation outside of the translation unit. In this case, treat it
// like any other out-of-line virtual method decl and export the record.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow this comment. Why is this the case? Are you referring to the ELF specific behaviour of the virtual dtor serving as a key function? If so, that is an itanium ABI specific behaviour. We can query the C++ model that is in use for evaluating this then.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, nothing ABI specific. I will rework this comment so it is more clear.

My intent was to explain that we have two cases for the implementation of a pure virtual destructor. Since pure virtual destructors are the only pure virtual methods that require an implementation (afaict), we know there must be one somewhere. If it is in the translation unit, we don't have to export it:

struct Example {
  virtual ~Example() = 0;
};

Example::Example() {}

If it is not in the translation unit, we assume the destructor implementation is in a .cpp file somewhere, so it does need to be exported.

struct Example {
  virtual ~Example() = 0
};

Alternatively: we could just ALWAYS export classes that have pure virtual functions. This may lead to exporting a bit more than necessary, but will also catch the case where a class has other pure virtual functions with implementations. Does that sound better?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the case now makes more sense! If we can get away with it, that would be nice ... but, doesn't the virtual dtor result in a vtable which also needs to be exported?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't the virtual dtor result in a vtable which also needs to be exported?

That's correct. This change results in a class getting exported whenever it has a pure virtual dtor.

Having given this more thought, I think we should stick with the more constrained case and export only when there is a pure virtual dtor. If we export a class whenever it contains any pure virtual function, that's going to export a lot more unnecessarily.

@andrurogerz andrurogerz deleted the pure-virtual-dtor branch July 8, 2025 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants