ExternalWorkload segmentation fault in 7.3

Last year we managed with @PierreZ to integrate a Rust Workload in the fdbserver simulation. We use the ExternalWorkload of FoundationDB to load a C++ wrapper that calls into Rust. It worked perfectly in version 7.1 but suddenly stopped in 7.3. We encounter a segmentation fault when instantiating our custom Workload. It happens before Rust is called, everything stays in “C++ land”. Here is the simplified code of our wrapper:

class MyWorkloadFactory: public FDBWorkloadFactory {
    public:
        MyWorkloadFactory(FDBLogger* logger): FDBWorkloadFactory() {
            logger->trace(FDBSeverity::Info, "MyWorkloadFactory", {});
        }

        virtual std::shared_ptr<FDBWorkload> create(const std::string& name) {
            // segmentation fault here:
            std::cout << "MyWorkloadFactory::create(" << name << ")" << std::endl;
            return std::make_shared<MyWorkload>(name);
        }
};

extern "C" FDBWorkloadFactory* workloadFactory(FDBLogger* logger) {
    static MyWorkloadFactory factory(logger);
    return &factory;
}

Here is the fdbserver call site from ExtertnalWorkload.actor.cpp:

FDBWorkloadFactory* (*workloadFactory)(FDBLogger*);
...
auto wName = ::getOption(options, "workloadName"_sr, ""_sr);
...
workloadFactory = reinterpret_cast<decltype(workloadFactory)>(loadFunction(library, "workloadFactory"));
if (workloadFactory == nullptr) {
	...
	return;
}
workloadImpl = (*workloadFactory)(FDBLoggerImpl::instance())->create(wName.toString());

I investigated all relevant header files for a meaningful change in contract but found nothing. Most of them haven’t even changed in years. I looked into the generated assembly and found that the fdbserer and our workload seem to represent the type const std::string& name differently. From the disassembly and gdb it seems to me that the fdbserver passes the const std::string& as a pointer in rdx which points to a memory region that looks like:

struct fdb_string {
	uint8_t size,
	char content[size/2],
}

This pointer seems to be 8 bytes aligned. Strangely, the size is double the length of the string and stored on a single byte. It feels more like a custom type than a type for std, but I couldn’t find anything in your Arena, StringRef and other similar types that matches exactly.

On our workload side, create seems to expect the string to be passed as a pointer of pointer in rdx. And the memory region pointed should look like:

struct w_string {
	uint64_t length,
	uint64_t _1,
	uint64_t _2,
	char content[length],
}

I don’t know what the 2 quadwords are for (it resembles a basic_string but I’m not sure). Strangely rdx is a double pointer indirection and doesn’t point to the start of the struct, but at the “content” field. The length is retrieved by looking 24 bytes before the value pointed by the pointer pointed by rdx.
For those interested, here is the simplified assembly of the workload trying to use the string:

mov    r13,rdx
mov    rsi,QWORD PTR [r13+0x0]	// &w_string.content
mov    rdx,QWORD PTR [rsi-0x18]	// *(&w_string.content-24) = w_string.length

This last instruction is the cause of the segfault, as the value rsi takes when the fdbserver calls is not a valid pointer.

Here is the simplified assembly of the fdbserver calling create:

test   rax,rax					// if (workloadFactory == nullptr)
je     0x39cacc6
lea    rdi,[rip+0x2310c8d]		// FDBLoggerImpl::instance()
call   rax						// (*workloadFactory)(logger)
mov    r13,rax					// workloadImpl
movsxd rbx,DWORD PTR [rbp-0x50]		// ???
lea    eax,[rbx+rbx*1]				// ???
mov    BYTE PTR [rbp-0x188],al		// fdb_string.size
mov    rax,QWORD PTR [r13+0x0]	// workloadImpl.vtable[0], FDBWorkloadFactory::create
lea    rdi,[rbp-0x90]			// ???
lea    rdx,[rbp-0x188]			// fdb_string
mov    rsi,r13					// "this" pointer of workloadImpl
call   QWORD PTR [rax]			// workloadImpl.create(fdb_string)

I tried to fix this issue by artificially reconstructing the string in my workload:

class MyWorkloadFactory: public FDBWorkloadFactory {
    public:
        ...
        virtual std::shared_ptr<FDBWorkload> create(const std::string& mangled) override {
            char* raw = (char*)&mangled;
            std::string name = std::string(raw+1, raw[0]>>1);
            return std::make_shared<WorkloadTranslater>(name);
        }
}

This works fine, I get back the expected name, but then I have another segmentation fault a bit later on FDBWorkloadContext::getOption also due to mismatched string representation.

I suspect this is due to a difference in compilation. We use the official FoundationDB docker (foundationdb/build:centos7-20240228040135-fc272dd89b) with devtoolset-11. What version of the docker and devtoolset did you use to compile the 7.3 fdbserver? Could it have changed between 7.1 and 7.3?

@ammolitor , is there any change of compilers for 7.3?

The 7.3 releases are compiled with clang. I think this would explain the discrepancy.

I looked into the gcc/clang differences and they both have their standard lib, respectively libstdc++ and libc++. Their string representation is indeed different and the pattern I found in the assembly seem to match their respect SSO (small string optimization).

I tried to compile with clang and libc++, but it always fails at linking:

  = note: /usr/bin/ld: cannot find -lc++
          collect2: error: ld returned 1 exit status

Here you can see ld comes from /usr/bin/, but it also fails if I source the devtoolset:

source /opt/rh/devtoolset-11/enable
cargo build -p foundationdb-simulation --release --example atomic --features fdb-7_3
...
  = note: /opt/rh/devtoolset-11/root/usr/libexec/gcc/x86_64-redhat-linux/11/ld: cannot find -lc++
          collect2: error: ld returned 1 exit status

Just in case, if I try to link against libstdc++, then it works with the devtoolset, and fails without:

  = note: /usr/bin/ld: cannot find -lstdc++
          collect2: error: ld returned 1 exit status

Do you use another linker and/or set the LD_LIBRARY_PATH (or other environment variables) to point to a specific standard library (/usr/local/include/c++/v1/ for example) to compile the fdbserver?

Ha, I found it, I used clang as linker and it finally works!
But I’m afraid it will become a recurring problem where each new version of the fdbserver could lead to a similar error because we must have the exact same compilation environment for our workload as the fdbserver. It is not a great situation as we want to rely more and more on the simulation.
May I suggest that the bindinds/c/foundationdb/ClientWorkload.h should expose pure C bindings instead of C++, like all the fdb_c*.h bindings? It would make integration in other languages a breeze and the standard C ABI would guarantee it will work regardless of the compilation environment of the workload. We won’t have to chase after the latest compilation environment.
As I worked on the Rust bindings I have some knowledge on the subject. Our integration is basically a C++ layer to convert into C bindings which then can be understood by Rust. I’m willing to make a PR that converts the C++ bindings in C and updates accordingly the ExternalWorkload, SimpleWorkload and JavaWorkload. Would you be interested?

1 Like

Yes. I think exposing a C interface is great. Can we keep the C++ interface as well? I.e., is it possible to wrap C++ with C ABI?

I got some time to try and implement a C API. You can find my attempt in this PR. As I didn’t want to change the ExternalWorkload or the JavaWorkload, I kept the C++ API unchanged and added the C bindings alongside.
I would really like some feedback on the solution or open discussions on other ways to solve the original problem.

I finally compiled the whole project and fixed all the compilation errors.
The system seems to work. Now the ExternalWorkload looks for a useCAPI option (alongside the libraryName, libraryPath…) to toggle between the C and C++ bindings. If the C API is used, the workloadInstantiate symbol is invoked (instead of workloadFactory) with a BridgeToServer instance (a collection of function pointers that enables C to interact with FDBWorkloadContext and GenericPromise<bool> pointers). The returned BridgeToClient (a collection of function pointers that enables C++ to interact with the C workload pointer) is wrapped in a “translator” C++ workload. So as far as the ExternalWorkload is concerned, only the instantiation is different, both API result in a C++ workload, so the rest of the code is unchanged.

Here is:

With this test file:

[[test]]
testTitle = 'MyCWorkloadTitle'

  [[test.workload]]
    testName = 'External'
    useCAPI = true
    libraryPath = './bin'
    libraryName = 'cworkload'
    workloadName = 'MyCWorkloadName'
    my_c_option = 'my_value'

I get the following output:

Run test:MyCWorkloadTitle start
ExternalWorkload::useCAPI: 1
workloadInstantiate(MyCWorkloadName, 0x7c596d08f120)[0/2]
my_c_option: my_value
workloadInstantiate(MyCWorkloadName, 0x7c5969b20540)[1/2]
my_c_option: my_value
setting up test (MyCWorkloadTitle)...
Test received trigger for setup...
workload_setup(MyCWorkloadName_0)
Test received trigger for setup...
workload_setup(MyCWorkloadName_1)
workload_start(MyCWorkloadName_0)
External complete
workload_start(MyCWorkloadName_1)
External complete
MyCWorkloadTitle complete
checking test (MyCWorkloadTitle)...
workload_check(MyCWorkloadName_1)
workload_getCheckTimeout(MyCWorkloadName_1)
workload_check(MyCWorkloadName_0)
workload_getCheckTimeout(MyCWorkloadName_0)
fetching metrics (MyCWorkloadTitle)...
workload_getMetrics(MyCWorkloadName_0)
workload_getMetrics(MyCWorkloadName_1)
workload_free(MyCWorkloadName_1)
workload_free(MyCWorkloadName_0)
FDBD joined cluster.
FDBD joined cluster.
setting up test (ConsistencyCheck)...
Test received trigger for setup...
Test received trigger for setup...
Set perpetual_storage_wiggle=0 ...
Set perpetual_storage_wiggle=0 Done.
Consistency scan already complete.
running test (ConsistencyCheck)...
ConsistencyCheck complete
ConsistencyCheck complete
ConsistencyCheck complete
ConsistencyCheck complete
ConsistencyCheck complete
checking test (ConsistencyCheck)...
fetching metrics (ConsistencyCheck)...
2 test clients passed; 0 test clients failed
Run test:MyCWorkloadTitle Done.

@jzhou I would appreciate it if you could review my PR.

1 Like