diff options
author | Jade Lovelace <lix@jade.fyi> | 2024-05-27 15:16:15 -0600 |
---|---|---|
committer | jade <lix@jade.fyi> | 2024-05-31 12:17:06 +0000 |
commit | ac78c1dcd501233339f6a2f2e67651c2eeac6498 (patch) | |
tree | 2429feeb0ba3c10c9edc7e5319bb8f09ae72d9d3 | |
parent | 0c6cb34de6033d0ceab5de8cbfc37465afeefaa4 (diff) |
libutil: fix args assert being thrown on Darwin in nix-eval-jobs
This is because a dynamic_cast<nix::RootArgs *> of a (n-e-j) MyArgs
returns nullptr even though MyArgs has virtual nix::RootArgs as a
parent.
class MyArgs : virtual public nix::MixEvalArgs,
virtual public nix::MixCommonArgs,
virtual nix::RootArgs { ... };
So this should work right?? But it does not. We found out that it's
caused by -fvisibility=hidden in n-e-j, but honestly this code was bad
anyway.
The trivial solution is to simply stop relying on RTTI working properly
here, which is probably better OO architecture anyway. However, I am not
100% confident *this* is sound, since we have this horrible hierarchy:
Args (defines getRoot)
/ | \
RootArgs MixCommonArgs MixEvalArgs
(overrides)
I am not confident that this is guaranteed to resolve from Args always
in the case of this override.
Assertion failed: (res), function getRoot, file src/libutil/args.cc, line 67.
6MyArgsProcess 60503 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = hit program assert
frame #4: 0x0000000100b1a41c liblixutil.dylib`nix::Args::processArgs(std::__1::list<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>> const&, bool) [inlined] nix::Args::getRoot(this=0x00000001000d0688) at args.cc:67:5 [opt]
64 std::cout << typeid(*p).name();
65
66 auto * res = dynamic_cast<RootArgs *>(p);
-> 67 assert(res);
68 return *res;
69 }
70
Target 0: (nix-eval-jobs) stopped.
(lldb) p this
(MyArgs *) 0x00000001000d0688
(lldb) p *this
(nix::Args) {
longFlags = size=180 { ... }
shortFlags = size=4 { ... }
expectedArgs = size=1 { ... }
processedArgs = size=0 {}
hiddenCategories = size=1 {
[0] = "Options to override configuration settings"
}
parent = nullptr
}
We also found that if we did this:
class [[gnu::visibility("default")]] RootArgs : virtual public Args
it would work properly (???!). This is of course, very strange, because
objdump -Ct output on liblixexpr.dylib is identical both with and
without it.
Possibly related: https://www.qt.io/blog/quality-assurance/one-way-dynamic_cast-across-library-boundaries-can-fail-and-how-to-fix-it
Fixes: https://git.lix.systems/lix-project/nix-eval-jobs/issues/2
Change-Id: I6b9ed968ed56420a9c4d2dffd18999d78c2761bd
-rw-r--r-- | src/libutil/args.cc | 2 | ||||
-rw-r--r-- | src/libutil/args.hh | 7 | ||||
-rw-r--r-- | src/libutil/args/root.hh | 4 |
3 files changed, 12 insertions, 1 deletions
diff --git a/src/libutil/args.cc b/src/libutil/args.cc index 655b3e82f..1342e7c6a 100644 --- a/src/libutil/args.cc +++ b/src/libutil/args.cc @@ -64,7 +64,7 @@ RootArgs & Args::getRoot() while (p->parent) p = p->parent; - auto * res = dynamic_cast<RootArgs *>(p); + auto res = p->asRootArgs(); assert(res); return *res; } diff --git a/src/libutil/args.hh b/src/libutil/args.hh index 35a5238c0..71f8f88fa 100644 --- a/src/libutil/args.hh +++ b/src/libutil/args.hh @@ -244,6 +244,13 @@ protected: */ virtual void initialFlagsProcessed() {} + /** + * Returns this Args as a RootArgs if it is one, or \ref std::nullopt otherwise. + */ + virtual std::optional<std::reference_wrapper<RootArgs>> asRootArgs() { + return std::nullopt; + } + public: void addFlag(Flag && flag); diff --git a/src/libutil/args/root.hh b/src/libutil/args/root.hh index f8124eaff..499ee6df4 100644 --- a/src/libutil/args/root.hh +++ b/src/libutil/args/root.hh @@ -65,6 +65,10 @@ protected: */ std::set<ExperimentalFeature> flagExperimentalFeatures; + virtual std::optional<std::reference_wrapper<RootArgs>> asRootArgs() override { + return *this; + } + private: std::optional<std::string> needsCompletion(std::string_view s); |