* [PR PATCH] fcitx5: fix crash when running configuration tools
@ 2021-07-07 14:25 sgn
2021-07-08 11:01 ` [PR PATCH] [Merged]: " sgn
0 siblings, 1 reply; 2+ messages in thread
From: sgn @ 2021-07-07 14:25 UTC (permalink / raw)
To: ml
[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]
There is a new pull request by sgn against master on the void-packages repository
https://github.com/sgn/void-packages fcitx5-fix-crash-config
https://github.com/void-linux/void-packages/pull/31843
fcitx5: fix crash when running configuration tools
<!-- Mark items with [x] where applicable -->
#### General
- [ ] This is a new package and it conforms to the [quality requirements](https://github.com/void-linux/void-packages/blob/master/Manual.md#quality-requirements)
#### Have the results of the proposed changes been tested?
- [ ] I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
- [ ] I generally don't use the affected packages but briefly tested this PR
<!--
If GitHub CI cannot be used to validate the build result (for example, if the
build is likely to take several hours), make sure to
[skip CI](https://github.com/void-linux/void-packages/blob/master/CONTRIBUTING.md#continuous-integration).
When skipping CI, uncomment and fill out the following section.
Note: for builds that are likely to complete in less than 2 hours, it is not
acceptable to skip CI.
-->
<!--
#### Does it build and run successfully?
(Please choose at least one native build and, if supported, at least one cross build. More are better.)
- [ ] I built this PR locally for my native architecture, (ARCH-LIBC)
- [ ] I built this PR locally for these architectures (if supported. mark crossbuilds):
- [ ] aarch64-musl
- [ ] armv7l
- [ ] armv6l-musl
-->
A patch file from https://github.com/void-linux/void-packages/pull/31843.patch is attached
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-fcitx5-fix-crash-config-31843.patch --]
[-- Type: text/x-diff, Size: 13222 bytes --]
From 58e9bcaa02e97ad2bc34263505b11fad2a981c4f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
<congdanhqx@gmail.com>
Date: Wed, 7 Jul 2021 19:35:07 +0700
Subject: [PATCH] fcitx5: fix crash when running configuration tools
---
...d-combinerType-to-signals-and-use-as.patch | 103 ++++++++++++++++++
...t-introduce-FCITX_DECLARE_SIGNAL_WIT.patch | 44 ++++++++
...combinerType-in-Signal-data-structur.patch | 76 +++++++++++++
srcpkgs/fcitx5/template | 2 +-
4 files changed, 224 insertions(+), 1 deletion(-)
create mode 100644 srcpkgs/fcitx5/patches/0001-SignalAdaptor-add-combinerType-to-signals-and-use-as.patch
create mode 100644 srcpkgs/fcitx5/patches/0002-connectableobject-introduce-FCITX_DECLARE_SIGNAL_WIT.patch
create mode 100644 srcpkgs/fcitx5/patches/0003-CheckUpdate-set-combinerType-in-Signal-data-structur.patch
diff --git a/srcpkgs/fcitx5/patches/0001-SignalAdaptor-add-combinerType-to-signals-and-use-as.patch b/srcpkgs/fcitx5/patches/0001-SignalAdaptor-add-combinerType-to-signals-and-use-as.patch
new file mode 100644
index 000000000000..53da7203e9dc
--- /dev/null
+++ b/srcpkgs/fcitx5/patches/0001-SignalAdaptor-add-combinerType-to-signals-and-use-as.patch
@@ -0,0 +1,103 @@
+From b9e666587c76386f5f4af38f87c0932af5e9e475 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
+ <congdanhqx@gmail.com>
+Date: Wed, 7 Jul 2021 18:28:05 +0700
+Subject: [PATCH 1/5] SignalAdaptor: add combinerType to signals, and use as
+ default combiner
+
+As of it's now, we're downcasting from SignalBase to
+Signal<SignalType> (which is the same type with
+Signal<SignalType, LastValue<...>>) in its member functions.
+
+The downcast is incorrect if its Combiner is NOT LastValue, fcitx5 will
+run into undefined behaviours with this kind of casting.
+
+In my local machine, this result in indeterminated value in
+CheckUpdateResult, then fcitx5 will crash because of DBus' assertion.
+
+When compiling with: -fsanitize=undefined, this problem is being
+reported:
+
+```
+../src/lib/fcitx/../fcitx-utils/connectableobject.h:115:18: runtime error: downcast of address 0x559bd4a2a5b0 which does not point to an object of type 'Signal'
+0x559bd4a2a5b0: note: object is of type '*N5fcitx6SignalIFbvENS_12_GLOBAL__N_117CheckUpdateResultEEE'
+ 00 00 00 00 40 25 21 2e f7 7f 00 00 e0 f5 a2 d4 9b 55 00 00 20 70 61 67 65 00 00 00 41 00 00 00
+ ^~~~~~~~~~~~~~~~~~~~~~~
+ vptr for '*N5fcitx6SignalIFbvENS_12_GLOBAL__N_117CheckUpdateResultEEE'
+../src/lib/fcitx/../fcitx-utils/connectableobject.h:116:21: runtime error: member call on address 0x559bd4a2a5b0 which does not point to an object of type 'Signal'
+0x559bd4a2a5b0: note: object is of type '*N5fcitx6SignalIFbvENS_12_GLOBAL__N_117CheckUpdateResultEEE'
+ 00 00 00 00 40 25 21 2e f7 7f 00 00 e0 f5 a2 d4 9b 55 00 00 20 70 61 67 65 00 00 00 41 00 00 00
+ ^~~~~~~~~~~~~~~~~~~~~~~
+ vptr for '*N5fcitx6SignalIFbvENS_12_GLOBAL__N_117CheckUpdateResultEEE'
+```
+
+As a preparatory step, let's add combinerType into data structure for
+Signal, and uses it as default combiner.
+---
+ src/lib/fcitx-utils/connectableobject.h | 21 ++++++++++-----------
+ 1 file changed, 10 insertions(+), 11 deletions(-)
+
+diff --git a/src/lib/fcitx-utils/connectableobject.h b/src/lib/fcitx-utils/connectableobject.h
+index b145ad3..1b0343d 100644
+--- a/src/lib/fcitx-utils/connectableobject.h
++++ b/src/lib/fcitx-utils/connectableobject.h
+@@ -20,10 +20,11 @@
+ /// \brief Utilities to enable use object with signal.
+
+ /// \brief Declare signal by type.
+-#define FCITX_DECLARE_SIGNAL(CLASS_NAME, NAME, ...) \
+- struct NAME { \
+- using signalType = __VA_ARGS__; \
+- using signature = fcitxMakeMetaString(#CLASS_NAME "::" #NAME); \
++#define FCITX_DECLARE_SIGNAL(CLASS_NAME, NAME, ...) \
++ struct NAME { \
++ using signalType = __VA_ARGS__; \
++ using combinerType = ::fcitx::LastValue<std::function<signalType>::result_type>; \
++ using signature = fcitxMakeMetaString(#CLASS_NAME "::" #NAME); \
+ }
+
+ /// \brief Declare a signal.
+@@ -53,8 +54,7 @@ namespace fcitx {
+ class ConnectableObject;
+
+ /// \brief Helper class to register class.
+-template <typename T, typename Combiner = LastValue<typename std::function<
+- typename T::signalType>::result_type>>
++template <typename T, typename Combiner = typename T::combinerType>
+ class SignalAdaptor {
+ public:
+ SignalAdaptor(ConnectableObject *d);
+@@ -79,7 +79,7 @@ public:
+ Connection connect(F &&func) {
+ auto signal = findSignal(SignalType::signature::data());
+ if (signal) {
+- return static_cast<Signal<typename SignalType::signalType> *>(
++ return static_cast<Signal<typename SignalType::signalType, typename SignalType::combinerType> *>(
+ signal)
+ ->connect(std::forward<F>(func));
+ }
+@@ -89,7 +89,7 @@ public:
+ template <typename SignalType>
+ void disconnectAll() {
+ auto signal = findSignal(SignalType::signature::data());
+- static_cast<Signal<typename SignalType::signalType> *>(signal)
++ static_cast<Signal<typename SignalType::signalType, typename SignalType::combinerType> *>(signal)
+ ->disconnectAll();
+ }
+
+@@ -112,13 +112,12 @@ protected:
+ template <typename SignalType, typename... Args>
+ auto emit(Args &&...args) const {
+ auto signal = findSignal(SignalType::signature::data());
+- return (*static_cast<Signal<typename SignalType::signalType> *>(
++ return (*static_cast<Signal<typename SignalType::signalType, typename SignalType::combinerType> *>(
+ signal))(std::forward<Args>(args)...);
+ }
+
+ template <typename SignalType,
+- typename Combiner = LastValue<typename std::function<
+- typename SignalType::signalType>::result_type>>
++ typename Combiner = typename SignalType::combinerType>
+ void registerSignal() {
+ _registerSignal(
+ SignalType::signature::data(),
diff --git a/srcpkgs/fcitx5/patches/0002-connectableobject-introduce-FCITX_DECLARE_SIGNAL_WIT.patch b/srcpkgs/fcitx5/patches/0002-connectableobject-introduce-FCITX_DECLARE_SIGNAL_WIT.patch
new file mode 100644
index 000000000000..255a09d5df23
--- /dev/null
+++ b/srcpkgs/fcitx5/patches/0002-connectableobject-introduce-FCITX_DECLARE_SIGNAL_WIT.patch
@@ -0,0 +1,44 @@
+From 7134470f681304d40d1756c31f20013a3911bf33 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
+ <congdanhqx@gmail.com>
+Date: Wed, 7 Jul 2021 19:12:54 +0700
+Subject: [PATCH 2/5] connectableobject: introduce
+ FCITX_DECLARE_SIGNAL_WITH_COMBINER
+
+In order to correctly downcast SignalBase, in the next step,
+we will switch all initialisation of SignalAdaptor to use
+SignalType::combinerType.
+
+Let's introduce a new macro to ease to initialisation of those
+signals which employs custom combiner.
+
+Arguably, the custom combiner is an implementation detail,
+thus it'll be put in source files. Hence, the macro is intended
+to be used outside of class declaration.
+---
+ src/lib/fcitx-utils/connectableobject.h | 12 ++++++++++++
+ 1 file changed, 12 insertions(+)
+
+diff --git a/src/lib/fcitx-utils/connectableobject.h b/src/lib/fcitx-utils/connectableobject.h
+index 1b0343d..1c318bf 100644
+--- a/src/lib/fcitx-utils/connectableobject.h
++++ b/src/lib/fcitx-utils/connectableobject.h
+@@ -27,6 +27,18 @@
+ using signature = fcitxMakeMetaString(#CLASS_NAME "::" #NAME); \
+ }
+
++/// \brief Declare signal by type with combiner.
++///
++/// This macro is intended to be used outside of class declaration,
++/// because the custom combiner is an implementation detail,
++/// thus it'll be put in source files.
++#define FCITX_DECLARE_SIGNAL_WITH_COMBINER(CLASS_NAME, NAME, COMBINER, ...) \
++ struct CLASS_NAME::NAME { \
++ using signalType = __VA_ARGS__; \
++ using combinerType = COMBINER; \
++ using signature = fcitxMakeMetaString(#CLASS_NAME "::" #NAME); \
++ }
++
+ /// \brief Declare a signal.
+ #define FCITX_DEFINE_SIGNAL(CLASS_NAME, NAME) \
+ ::fcitx::SignalAdaptor<CLASS_NAME::NAME> CLASS_NAME##NAME##Adaptor { this }
diff --git a/srcpkgs/fcitx5/patches/0003-CheckUpdate-set-combinerType-in-Signal-data-structur.patch b/srcpkgs/fcitx5/patches/0003-CheckUpdate-set-combinerType-in-Signal-data-structur.patch
new file mode 100644
index 000000000000..24d965ab144f
--- /dev/null
+++ b/srcpkgs/fcitx5/patches/0003-CheckUpdate-set-combinerType-in-Signal-data-structur.patch
@@ -0,0 +1,76 @@
+From aeafbe384649f06ef24380167ec3bdb876c44acb Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
+ <congdanhqx@gmail.com>
+Date: Wed, 7 Jul 2021 18:28:05 +0700
+Subject: [PATCH 3/5] CheckUpdate: set combinerType in Signal data structure to
+ CheckUpdateResult
+
+As of it's now, we're downcasting Signal data structure for CheckUpdate
+from SignalBase to Signal<bool()> (which is the same type with
+Signal<bool(), LastValue<bool>>) in SignalAdaptor member functions.
+
+The downcast is incorrect because it's Signal<bool(), CheckUpdateResult>
+instead, thus fcitx5 will run into undefined behaviours with this downcasting.
+
+In my local machine, this result in indeterminated value in
+CheckUpdateResult, then fcitx5 will crash because of DBus' assertion.
+
+When compiling with: -fsanitize=undefined, this problem is being
+reported:
+
+```
+../src/lib/fcitx/../fcitx-utils/connectableobject.h:115:18: runtime error: downcast of address 0x559bd4a2a5b0 which does not point to an object of type 'Signal'
+0x559bd4a2a5b0: note: object is of type '*N5fcitx6SignalIFbvENS_12_GLOBAL__N_117CheckUpdateResultEEE'
+ 00 00 00 00 40 25 21 2e f7 7f 00 00 e0 f5 a2 d4 9b 55 00 00 20 70 61 67 65 00 00 00 41 00 00 00
+ ^~~~~~~~~~~~~~~~~~~~~~~
+ vptr for '*N5fcitx6SignalIFbvENS_12_GLOBAL__N_117CheckUpdateResultEEE'
+../src/lib/fcitx/../fcitx-utils/connectableobject.h:116:21: runtime error: member call on address 0x559bd4a2a5b0 which does not point to an object of type 'Signal'
+0x559bd4a2a5b0: note: object is of type '*N5fcitx6SignalIFbvENS_12_GLOBAL__N_117CheckUpdateResultEEE'
+ 00 00 00 00 40 25 21 2e f7 7f 00 00 e0 f5 a2 d4 9b 55 00 00 20 70 61 67 65 00 00 00 41 00 00 00
+ ^~~~~~~~~~~~~~~~~~~~~~~
+ vptr for '*N5fcitx6SignalIFbvENS_12_GLOBAL__N_117CheckUpdateResultEEE'
+```
+
+Let's downcast it into correct type by setting CheckUpdateResult as
+combinerType.
+---
+ src/lib/fcitx/instance.cpp | 5 +++--
+ src/lib/fcitx/instance.h | 2 +-
+ 2 files changed, 4 insertions(+), 3 deletions(-)
+
+diff --git a/src/lib/fcitx/instance.cpp b/src/lib/fcitx/instance.cpp
+index b672383..66bbd51 100644
+--- a/src/lib/fcitx/instance.cpp
++++ b/src/lib/fcitx/instance.cpp
+@@ -126,6 +126,8 @@ std::string stripLanguage(const std::string &lc) {
+
+ } // namespace
+
++FCITX_DECLARE_SIGNAL_WITH_COMBINER(Instance, CheckUpdate, CheckUpdateResult, bool());
++
+ class CheckInputMethodChanged;
+
+ struct InputState : public InputContextProperty {
+@@ -455,8 +457,7 @@ public:
+ FCITX_DEFINE_SIGNAL_PRIVATE(Instance, CommitFilter);
+ FCITX_DEFINE_SIGNAL_PRIVATE(Instance, OutputFilter);
+ FCITX_DEFINE_SIGNAL_PRIVATE(Instance, KeyEventResult);
+- FCITX_DEFINE_SIGNAL_PRIVATE_WITH_COMBINER(Instance, CheckUpdate,
+- CheckUpdateResult);
++ FCITX_DEFINE_SIGNAL_PRIVATE(Instance, CheckUpdate);
+
+ FactoryFor<InputState> inputStateFactory_{
+ [this](InputContext &ic) { return new InputState(this, &ic); }};
+diff --git a/src/lib/fcitx/instance.h b/src/lib/fcitx/instance.h
+index a3355f0..139ff51 100644
+--- a/src/lib/fcitx/instance.h
++++ b/src/lib/fcitx/instance.h
+@@ -127,7 +127,7 @@ public:
+ void(InputContext *inputContext, Text &orig));
+ FCITX_DECLARE_SIGNAL(Instance, KeyEventResult,
+ void(const KeyEvent &keyEvent));
+- FCITX_DECLARE_SIGNAL(Instance, CheckUpdate, bool());
++ struct CheckUpdate;
+
+ /// Return a focused input context.
+ InputContext *lastFocusedInputContext();
diff --git a/srcpkgs/fcitx5/template b/srcpkgs/fcitx5/template
index daedc89f5b3e..077b99b54ab6 100644
--- a/srcpkgs/fcitx5/template
+++ b/srcpkgs/fcitx5/template
@@ -1,7 +1,7 @@
# Template file for 'fcitx5'
pkgname=fcitx5
version=5.0.8
-revision=1
+revision=2
build_style=cmake
build_helper=qemu
configure_args="
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PR PATCH] [Merged]: fcitx5: fix crash when running configuration tools
2021-07-07 14:25 [PR PATCH] fcitx5: fix crash when running configuration tools sgn
@ 2021-07-08 11:01 ` sgn
0 siblings, 0 replies; 2+ messages in thread
From: sgn @ 2021-07-08 11:01 UTC (permalink / raw)
To: ml
[-- Attachment #1: Type: text/plain, Size: 1461 bytes --]
There's a merged pull request on the void-packages repository
fcitx5: fix crash when running configuration tools
https://github.com/void-linux/void-packages/pull/31843
Description:
<!-- Mark items with [x] where applicable -->
#### General
- [ ] This is a new package and it conforms to the [quality requirements](https://github.com/void-linux/void-packages/blob/master/Manual.md#quality-requirements)
#### Have the results of the proposed changes been tested?
- [x] I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
- [ ] I generally don't use the affected packages but briefly tested this PR
<!--
If GitHub CI cannot be used to validate the build result (for example, if the
build is likely to take several hours), make sure to
[skip CI](https://github.com/void-linux/void-packages/blob/master/CONTRIBUTING.md#continuous-integration).
When skipping CI, uncomment and fill out the following section.
Note: for builds that are likely to complete in less than 2 hours, it is not
acceptable to skip CI.
-->
<!--
#### Does it build and run successfully?
(Please choose at least one native build and, if supported, at least one cross build. More are better.)
- [ ] I built this PR locally for my native architecture, (ARCH-LIBC)
- [ ] I built this PR locally for these architectures (if supported. mark crossbuilds):
- [ ] aarch64-musl
- [ ] armv7l
- [ ] armv6l-musl
-->
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-07-08 11:01 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07 14:25 [PR PATCH] fcitx5: fix crash when running configuration tools sgn
2021-07-08 11:01 ` [PR PATCH] [Merged]: " sgn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).