Move AppManager::getEnabledApps_ to a .cpp file to prevent multiple definitions

[bdutro-mips]

I ran into an issue trying to compile our internal simulator with clang and link it against a Sparta + SimDB compiled with GCC. Clang was instantiating its own instance of AppManager::getEnabledApps_ because it was present in the header and not linking against the instance in libsparta.a.

It seems that the singleton pattern requires the static instance to live in a single translation unit to guarantee that it works correctly across linkers.

I threw this together quickly, so let me know if the CMake config needs any changes.

[colby-nyce]

I need to understand how to repro this myself, as it's not obvious to me what's going on. Static methods / local static variables in methods in header files is a common thing. What's the minimum I need to reproduce this? I'm guessing there are build/linker steps involved that I am not thinking of.

Side note: I've tried very hard to keep SimDB as a header-only library, and I want to keep it that way. I'm doubting that we need a full .cpp file just for this.

Are you using the installed Sparta in the conda env sparta2?
What machine are you building this on?
Does a clean rebuild do anything?
Are you using SPARTA_SEARCH_DIR?
Are you mixing compilers? Sparta compiled with one, internal simulators compiled with another...
What cmake command did you use? (-DCMAKE....)
Can you attach CMakeCache.txt?

[bdutro-mips]

Repro instructions:

1. Build Sparta with GCC and install it to a prefix:

CC=gcc CXX=g++ cmake .. -DCMAKE_BUILD_TYPE=Release
cmake --install . --prefix <SPARTA_DIR>

2. Build pegasus with Clang and the Sparta we just built:

CC=clang CXX=clang++ cmake .. -DCMAKE_BUILD_TYPE=Release -DSPARTA_SEARCH_DIR=<SPARTA_DIR>

3. Run Pegasus regression:

make -j16 pegasus_regress

4. Two tests fail:

92% tests passed, 2 tests failed out of 26

Total Test time (real) =  19.12 sec

The following tests FAILED:
         22 - CoSimFlushWorkload_test_run (Failed)
         23 - CoSimFlushSysCall_test_run (Failed)

Here's the error message:

Command: "/home/bdutro/work/pegasus-public/release_clang/test/cosim/cosim_workload/flush_workload/FlushWorkload_test" "-w" "rv64mi-p-csr" "-p" "top.core0.params.isa" "rv64imafdcbv_zicsr_zifencei_zicond_zfh"
Directory: /home/bdutro/work/pegasus-public/release_clang/test/cosim/cosim_workload/flush_workload
"CoSimFlushWorkload_test_run" start time: Feb 17 10:14 PST
Output:
----------------------------------------------------------
  [in] Configuration: Parameter "top.core0.params.isa" <- value: "rv64imafdcbv_zicsr_zifencei_zicond_zfh"
  [in] Configuration: Parameter "top.extension.sim.workloads" <- value: "[[rv64mi-p-csr]]"
  [in] Configuration: Parameter "top.extension.sim.inst_limit" <- value: "0"
  [out] placing tap on node top for: Tap location_pattern="top" (category="inst") -> file: "rv64mi-p-csr.log"
Building tree...
Configuring tree...
Finalizing tree...

Loading ELF binary: rv64mi-p-csr
Found magic memory symbols in ELF
    tohost:   0x80001000
    fromhost: 0x80001040
Automatically constructing magic memory at 0x80001000
map: [         0,         0x80001000) -> "        mb_1" +0x0
map: [0x80001000,         0x80002000) -> "Magic Memory" +0x0
map: [0x80002000, 0x8000000000000000) -> "        mb_2" +0x0
  -- Loading section  (99B)  to 0x0
  -- Loading section .text.init (1152B)  to 0x480
  -- Loading section .tohost (4112B)  to 0x1010
Starting PC: 0x80000000
PegasusExtractorAllocatorWrapper: 5d0 objects allocated/created
PegasusExtractorAllocatorWrapper: 5d0 objects allocated/created
PegasusInstAllocatorWrapper: 0 objects allocated/created
PegasusExtractorAllocatorWrapper: 5d0 objects allocated/created
PegasusInstAllocatorWrapper: 0 objects allocated/created
    NOTE: unread optional unbound parameter: "top.extension.sim.inst_limit" from: "command line". value: "0". Path exists in tree up to: "top"
    NOTE: unread optional unbound parameter: "top.extension.sim.workloads" from: "command line". value: "[[rv64mi-p-csr]]". Path exists in tree up to: "top"
Booting hartid 0
PegasusState::boot()
        MHARTID: 0
        MISA:    800000000034112f
        MSTATUS: a00000000
        SSTATUS: 200000000
  [in] Configuration: Parameter "top.core0.params.isa" <- value: "rv64imafdcbv_zicsr_zifencei_zicond_zfh"
  [in] Configuration: Parameter "top.extension.sim.inst_limit" <- value: "0"
  [in] Configuration: Parameter "top.extension.sim.workloads" <- value: "[[rv64mi-p-csr]]"
  [in] Configuration: Parameter "top.core*.params.cosim_mode" <- value: "true"
Building tree...
Configuring tree...
Finalizing tree...

Loading ELF binary: rv64mi-p-csr
Found magic memory symbols in ELF
    tohost:   0x80001000
    fromhost: 0x80001040
Automatically constructing magic memory at 0x80001000
map: [         0,         0x80001000) -> "        mb_1" +0x0
map: [0x80001000,         0x80002000) -> "Magic Memory" +0x0
map: [0x80002000, 0x8000000000000000) -> "        mb_2" +0x0
  -- Loading section  (99B)  to 0x0
  -- Loading section .text.init (1152B)  to 0x480
  -- Loading section .tohost (4112B)  to 0x1010
Starting PC: 0x80000000
PegasusExtractorAllocatorWrapper: 5d0 objects allocated/created
PegasusExtractorAllocatorWrapper: 5d0 objects allocated/created
PegasusInstAllocatorWrapper: 0 objects allocated/created
PegasusExtractorAllocatorWrapper: 5d0 objects allocated/created
PegasusInstAllocatorWrapper: 0 objects allocated/created
    NOTE: unread optional unbound parameter: "top.extension.sim.workloads" from: "command line". value: "[[rv64mi-p-csr]]". Path exists in tree up to: "top"
    NOTE: unread optional unbound parameter: "top.extension.sim.inst_limit" from: "command line". value: "0". Path exists in tree up to: "top"
terminate called after throwing an instance of 'simdb::DBException'
  what():  You need to call enableApp() before parameterizing factories.

5. If you look at the symbols in test/cosim/cosim_workload/flush_workload/FlushWorkload_test, you'll see the problem:

nm -C FlushWorkload_test | grep EnabledApps
0000000000b434a0 u guard variable for simdb::AppManager::getEnabledApps_[abi:cxx11]()::enabled_apps
0000000000b46e48 V guard variable for simdb::AppManager::getEnabledApps_[abi:cxx11]()::enabled_apps[abi:cxx11]
0000000000310b60 t simdb::AppManager::createEnabledApps_()
0000000000b434c0 u simdb::AppManager::getEnabledApps_[abi:cxx11]()::enabled_apps
0000000000b46e18 V simdb::AppManager::getEnabledApps_[abi:cxx11]()::enabled_apps[abi:cxx11]

[bdutro-mips]

I've tested this with these Clang and GCC versions and get the same problem every time:

• GCC 12.3.0 and Clang 15.0.7
• GCC 13.3.0 and Clang 19.1.7
• GCC 15.1.0 and Clang 20.1.8
• GCC 15.2.1 and Clang 21.1.8

[bdutro-mips]

I'll also note that this bug shows up in some executables but not others - I think Clang is making different inlining decisions that affect whether the duplicate symbol shows up and/or whether it's a problem.

[colby-nyce]

Let's wait until the tree node extensions redesign gets into map_v2.1 and try this again. But I'm 99% sure that this has nothing to do with SimDB specifically -- the use of SPARTA_SEARCH_DIR combined with Conda is broken. I'm of the opinion that SPARTA_SEARCH_DIR should not be used for our internal simulators, and they all should build Sparta from source, OR they can only use an "official" Sparta install in Conda. So like this:

cmake .. -DBUILD_SPARTA_FROM_SOURCE=ON # defaults to OFF

Which will do the local git clone --recursive ... local_sparta and then add_subdirectory(local_sparta)

When I tried your repro steps using a debug build, even with a map_v2.1 install in /home/cnyce/map-brett, when I step through the code it gets into source files in $CONDA_PREFIX. So we either need to figure out the real way to do SPARTA_SEARCH_DIR or remove it and build Sparta from source.

Mixing-and-matching compilers for Sparta and downstream simulators is not helping either -- yet another reason to build from source.

[bdutro-mips]

When I tried your repro steps using a debug build, even with a map_v2.1 install in /home/cnyce/map-brett, when I step through the code it gets into source files in $CONDA_PREFIX. So we either need to figure out the real way to do SPARTA_SEARCH_DIR or remove it and build Sparta from source.

Yeah, you have to override quite a few CMake variables to get it to ignore any centrally installed Sparta builds. However I also reproduced this bug on systems with no Conda install and no global Sparta.

[bdutro-mips]

Mixing-and-matching compilers for Sparta and downstream simulators is not helping either -- yet another reason to build from source.

Yeah, if we want to advertise Sparta as a truly packageable library then we need to get this fixed, but I'm also ok with building from source every time (it doesn't take that long to build compared to the simulators that use it).

[colby-nyce]

I fixed this and merged it in:
#171

In the short term, we'll just use these non-static apis. In the longer term, I have a design for a real plugin system using shared libraries that won't have any static registration / static apis. But this quick workaround will do for now.

I'm going to update Sparta to use this new stuff and add the changes to my larger open PR for tree node extensions. Then I'll make any changes needed for internal simulators after that.