options/internal: fix compilation on clang

[no92]

Previously, guarding the different operator deletes was done in the name of different mangling, which according to c++filt is not the case. To rectify this, both operator deletes will now get compiled in, as they do take different arguments.

[ArsenArsen]

I'm not a fan of this solution, it involves runtime warnings for something that can definitely be detected at link time (or better). I'll see if there's something better that we can do (e.g. poisoning these functions and telling LD to give up on them).

Also, a better way to catch both GCC and Clang is to define all overloads of these functions, probably (see libsupc++)

[ArsenArsen]

okay, #pragma GCC poison delete will block all instances of delete that follow it, so if we use that in common (internal) headers, that should be okay; however, that doesn't work for new (since it'd conflict with placement new), so I'm looking for further solutions

[ArsenArsen]

here's a different fix that also covers other cases (note that I haven't tested it on clang, but I don't see why it wouldn't work)

From 420d8f2aed8a84a9903820229b29fdf3b5e8c727 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Arsen=20Arsenovi=C4=87?= <[email protected]>
Date: Wed, 19 Oct 2022 11:40:52 +0200
Subject: [PATCH] options/internal: enable the use of -z defs

note: don't apply this commit without splitting it, there's two changes
here (removing instances of operator delete(...) in deleting destructors
in virtual classes and related hacks, and then enabling defs)

The member operator deletes are defined to prevent the default of
calling libsupc++'s version being called (see the Itanium ABI[1], "The
entries for virtual destructors ...").

[1] https://itanium-cxx-abi.github.io/cxx-abi/abi.html
---
 meson.build                                   |  2 +-
 options/ansi/include/mlibc/file-io.hpp        |  4 ++++
 options/internal/gcc-extra/cxxabi.cpp         | 16 ----------------
 options/internal/generic/charcode.cpp         |  2 ++
 options/internal/include/mlibc/charcode.hpp   |  2 ++
 options/posix/generic/posix_stdio.cpp         |  2 ++
 options/posix/include/mlibc/posix-file-io.hpp |  2 ++
 7 files changed, 13 insertions(+), 17 deletions(-)
 delete mode 100644 options/internal/gcc-extra/cxxabi.cpp

diff --git a/meson.build b/meson.build
index 131415da..bfbbc00c 100644
--- a/meson.build
+++ b/meson.build
@@ -60,6 +60,7 @@ if not headers_only
 	add_project_arguments('-nostdinc', '-fno-builtin', language: ['c', 'cpp'])
 	add_project_arguments('-fno-rtti', '-fno-exceptions', language: 'cpp')
 	add_project_link_arguments('-nostdlib', language: ['c', 'cpp'])
+	add_project_link_arguments('-Wl,-z,defs', language: ['c', 'cpp'])
 
 	searchdirs = run_command(c_compiler.cmd_array(), '-print-search-dirs',
 				check: true).stdout()
@@ -231,7 +232,6 @@ internal_sources = [
 	'options/internal/gcc/stack_protector.cpp',
 	'options/internal/gcc/guard-abi.cpp',
 	'options/internal/gcc/initfini.cpp',
-	'options/internal/gcc-extra/cxxabi.cpp',
 	'options/internal' / host_machine.cpu_family() / 'setjmp.S',
 ]
 
diff --git a/options/ansi/include/mlibc/file-io.hpp b/options/ansi/include/mlibc/file-io.hpp
index 15b2c192..85a7c17a 100644
--- a/options/ansi/include/mlibc/file-io.hpp
+++ b/options/ansi/include/mlibc/file-io.hpp
@@ -48,6 +48,8 @@ public:
 	int tell(off_t *current_offset);
 	int seek(off_t offset, int whence);
 
+	void operator delete(void*) { __builtin_trap(); }
+
 protected:
 	virtual int determine_type(stream_type *type) = 0;
 	virtual int determine_bufmode(buffer_mode *mode) = 0;
@@ -85,6 +87,8 @@ struct fd_file : abstract_file {
 
 	static int parse_modestring(const char *mode);
 
+	void operator delete(void*) { __builtin_trap(); }
+
 protected:
 	int determine_type(stream_type *type) override;
 	int determine_bufmode(buffer_mode *mode) override;
diff --git a/options/internal/gcc-extra/cxxabi.cpp b/options/internal/gcc-extra/cxxabi.cpp
deleted file mode 100644
index aa222a83..00000000
--- a/options/internal/gcc-extra/cxxabi.cpp
+++ /dev/null
@@ -1,16 +0,0 @@
-
-#include <bits/ensure.h>
-
-// The cxxabi needs operator delete for *deleting* destructors, i.e., destructors that
-// are called by delete expressions. We never use such expressions in mlibc.
-// Note that G++ complains if we make the operator hidden,
-// thus we use it's mangled name as a workaround.
-#if defined(__clang__)
-	extern "C" [[gnu::visibility("hidden")]] void _ZdlPv() { // operator delete (void *, size_t)
-		__ensure(!"operator delete called! delete expressions cannot be used in mlibc.");
-	}
-#else
-	extern "C" [[gnu::visibility("hidden")]] void _ZdlPvm() { // operator delete (void *, size_t)
-		__ensure(!"operator delete called! delete expressions cannot be used in mlibc.");
-	}
-#endif
\ No newline at end of file
diff --git a/options/internal/generic/charcode.cpp b/options/internal/generic/charcode.cpp
index e09d5cd2..49dc1597 100644
--- a/options/internal/generic/charcode.cpp
+++ b/options/internal/generic/charcode.cpp
@@ -222,6 +222,8 @@ struct polymorphic_charcode_adapter : polymorphic_charcode {
 
 		return charcode_error::null;
 	}
+
+	void operator delete(void*) { __builtin_trap(); }
 };
 
 polymorphic_charcode *current_charcode() {
diff --git a/options/internal/include/mlibc/charcode.hpp b/options/internal/include/mlibc/charcode.hpp
index 67bd03d9..e666145a 100644
--- a/options/internal/include/mlibc/charcode.hpp
+++ b/options/internal/include/mlibc/charcode.hpp
@@ -107,6 +107,8 @@ struct polymorphic_charcode {
 
 	// Whether the encoding has shift states.
 	const bool has_shift_states;
+
+	void operator delete(void*) { __builtin_trap(); }
 };
 
 polymorphic_charcode *current_charcode();
diff --git a/options/posix/generic/posix_stdio.cpp b/options/posix/generic/posix_stdio.cpp
index f38dc658..d705735e 100644
--- a/options/posix/generic/posix_stdio.cpp
+++ b/options/posix/generic/posix_stdio.cpp
@@ -24,6 +24,8 @@ struct popen_file : mlibc::fd_file {
 		_popen_pid = new_pid;
 	}
 
+	void operator delete(void*) { __builtin_trap(); }
+
 private:
 	// Underlying PID in case of popen()
 	pid_t _popen_pid;
diff --git a/options/posix/include/mlibc/posix-file-io.hpp b/options/posix/include/mlibc/posix-file-io.hpp
index dd38fa82..e375b2e6 100644
--- a/options/posix/include/mlibc/posix-file-io.hpp
+++ b/options/posix/include/mlibc/posix-file-io.hpp
@@ -11,6 +11,8 @@ struct mem_file : abstract_file {
 	mem_file(char **ptr, size_t *sizeloc, void (*do_dispose)(abstract_file *) = nullptr);
 
 	int close() override;
+
+	void operator delete(void*) { __builtin_trap(); }
 protected:
 	int determine_type(stream_type *type) override;
 	int determine_bufmode(buffer_mode *mode) override;
-- 
2.38.0

[Dennisbonke]

I'm curious to find out what @avdgrinten @Geertiebear @qookei think of this. Which approach should we pick.

[no92]

This PR is ready now, but the Github UI looks funny due to the fact that the required jobs were changed (by adding the compiler to the matrix).

[no92]

I don't see the original issue (the operator delete stuff) addressed anywhere in this PR?

That got dropped as I didn't run into that issue any longer.

[no92]

Addressed the issues raised.