From ac78c1dcd501233339f6a2f2e67651c2eeac6498 Mon Sep 17 00:00:00 2001
From: Jade Lovelace <lix@jade.fyi>
Date: Mon, 27 May 2024 15:16:15 -0600
Subject: [PATCH] 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
---
 src/libutil/args.cc      | 2 +-
 src/libutil/args.hh      | 7 +++++++
 src/libutil/args/root.hh | 4 ++++
 3 files changed, 12 insertions(+), 1 deletion(-)

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);