Skip to content

ir: Choose the right mangling for destructors on all codepaths. #1240

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

Merged
merged 1 commit into from
Jan 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions bindgen-integration/cpp/Test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,18 @@
const int Test::COUNTDOWN[] = { 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 };
const int* Test::COUNTDOWN_PTR = Test::COUNTDOWN;

unsigned VirtualDestructor::sDestructorCount = 0;
VirtualDestructor::~VirtualDestructor() {
sDestructorCount++;
}

unsigned InheritsFromVirtualDestructor::sDestructorCount = 0;
InheritsFromVirtualDestructor::InheritsFromVirtualDestructor() = default;

InheritsFromVirtualDestructor::~InheritsFromVirtualDestructor() {
sDestructorCount++;
}

const int* Test::countdown() {
return COUNTDOWN;
}
Expand Down
13 changes: 13 additions & 0 deletions bindgen-integration/cpp/Test.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,19 @@ class ITest {
virtual void foo() = 0;
};

class VirtualDestructor {
public:
static unsigned sDestructorCount;
virtual ~VirtualDestructor() = 0;
};

class InheritsFromVirtualDestructor final : public VirtualDestructor {
public:
static unsigned sDestructorCount;
InheritsFromVirtualDestructor();
~InheritsFromVirtualDestructor() final;
};

namespace testing {

typedef Test TypeAlias;
Expand Down
19 changes: 19 additions & 0 deletions bindgen-integration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,3 +260,22 @@ fn test_destructors() {

assert!(v, "Should've been restored when going out of scope");
}

impl Drop for bindings::InheritsFromVirtualDestructor {
fn drop(&mut self) {
unsafe { bindings::InheritsFromVirtualDestructor_InheritsFromVirtualDestructor_destructor(self) }
}
}

#[test]
fn test_virtual_dtor() {
unsafe {
{
let b = bindings::InheritsFromVirtualDestructor::new();
// Let it go out of scope.
}

assert_eq!(bindings::InheritsFromVirtualDestructor_sDestructorCount, 1);
assert_eq!(bindings::VirtualDestructor_sDestructorCount, 1);
}
}
1 change: 1 addition & 0 deletions src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1985,6 +1985,7 @@ impl MethodCodegen for Method {
}
});

// TODO(emilio): We could generate final stuff at least.
if self.is_virtual() {
return; // FIXME
}
Expand Down
11 changes: 9 additions & 2 deletions src/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ pub fn cursor_mangling(
cursor: &clang::Cursor,
) -> Option<String> {
use clang_sys;

if !ctx.options().enable_mangling {
return None;
}
Expand All @@ -256,8 +257,14 @@ pub fn cursor_mangling(
return None;
}

let is_destructor = cursor.kind() == clang_sys::CXCursor_Destructor;
if let Ok(mut manglings) = cursor.cxx_manglings() {
if let Some(m) = manglings.pop() {
while let Some(m) = manglings.pop() {
// Only generate the destructor group 1, see below.
if is_destructor && !m.ends_with("D1Ev") {
continue;
}

return Some(m);
}
}
Expand All @@ -267,7 +274,7 @@ pub fn cursor_mangling(
return None;
}

if cursor.kind() == clang_sys::CXCursor_Destructor {
if is_destructor {
// With old (3.8-) libclang versions, and the Itanium ABI, clang returns
// the "destructor group 0" symbol, which means that it'll try to free
// memory, which definitely isn't what we want.
Expand Down
4 changes: 1 addition & 3 deletions tests/expectations/tests/virtual_dtor.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
/* automatically generated by rust-bindgen */


#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]


#[repr(C)]
pub struct nsSlots__bindgen_vtable(::std::os::raw::c_void);
#[repr(C)]
Expand All @@ -30,6 +28,6 @@ impl Default for nsSlots {
}
}
extern "C" {
#[link_name = "\u{1}_ZN7nsSlotsD0Ev"]
#[link_name = "\u{1}_ZN7nsSlotsD1Ev"]
pub fn nsSlots_nsSlots_destructor(this: *mut nsSlots);
}