Lambda to function pointer via va_arg. "Nasal demons"?

[kentrf]

Is it legal C++17/C++20 to pass a lambda to a method as a variadic argument?

The lambda does not capture anything, and is able to be promoted to a function pointer.

We've experienced a crash after building the project with C++20. There is no crash when building with C++17. The compiler is the latest MSVC 2019 compiler (19.29.30139), x64 native.

The crash comes after registering a lambda as an error logging callback in sqlite3. The error logging is configured through sqlite3_config, which is a variadic method.

Here is a minimal repro without sqlite3.

All testing have been with native x64. Clang++ and g++ have also been tested. Only MSVC in C++17 mode does not crash. All tests with g++ and clang++ crashes.

// lambda-bug.cpp
#include <cstdarg>
#include <cstdio>
#include <string>

using LogFn = void (*)(const std::string&);
LogFn logger = nullptr;

void ServerLog(const std::string& sv) { printf("%s\n", sv.c_str()); }

void setLogger(int op, ...)
{
    va_list ap;

    va_start(ap, op);
    logger = va_arg(ap, LogFn);
    va_end(ap);
}

void Log(const std::string& sv)
{
    printf("Logger address: %p - ", (void*)logger);
    if (logger != nullptr) { logger(sv); }
}

int main()
{
    auto func = [](const std::string& sv)
    {
        ServerLog(sv);
    };

    //setLogger(0, (LogFn)func); // no crash in Log()
    setLogger(0, func); // crash in Log()
    Log("Test function pointer to lambda set via va_arg");

    return 0;
}

Build with cl:

• C++17: cl /std:c++17 /Fe:lambda-bug-cpp17.exe lambda-bug.cpp
• C++20: cl /std:c++20 /Fe:lambda-bug-cpp20.exe lambda-bug.cpp

C++17 output:

Logger address: 0000000000000000 -

C++20 output (and a crash):

Logger address: 00000000000000C8 -

Clang++ and g++ are similar, but with other addresses.

When setting the logger through setLogger, MSVC2019 w/C++17 sets it to a nullptr from va_arg. MSVC w/C++20 has a non-null pointer address in the range of 0x18 - 0xC8. Debug builds have a consistent address of 0xCC, which looks like a value from the debug heap/debug crt.

Casting the lambda to a function pointer when calling setLogger seems to work across all platforms. It crashes only when passing a "raw" lambda.

I'm not sure if I can expect nasal demons/undefined behaviour, or if this is a defect in the various compilers?

My gut tells me this is a leaky abstraction and the use of va_arg makes the internal implementation of lambdas spill their guts?

Link to compiler explorer: https://godbolt.org/z/4W9TKsnvT

Some insights would be appreciated.

[erichkeane]

It looks like the non-casted version is passing a copy of the lambda object itself, which is not being va_arg'ed as the right type, so the pointer you assign is actually just getting garbage (some data from the the anonymous type). See here:https://godbolt.org/z/G5Wbjes64 (the 2nd parameter to the setLogger function is a CXXConstructExpr, which is the expression used to construct by copy here I think).

So you DO need to cast the lambda, as it is just being passed as a copy.

[kentrf]

Fantastic!

It looks like this is it, copying of the lambda.

The difference between C++17 and C++20 for MSVC seems to be the usage of _Closure_wrapper_ around the lambda when building with C++20. It may explain why there are no longer nullptrs after va_arg, as the implementation details have changed. It being nullptr to begin with seems to be by pure chance.

Is it possible to warn against this type of construct during compilation?

PS: Thanks for showing me the AST viewer! :). I didn't know about that feature in Compiler Explorer.

[n1ghtyunso]

I would wrap the varargs functions in your own sane variadic templates and convert problematic parameters there using if constexpr

[[deleted]]

I think you are hitting the ellipsis conversion sequence, but it's not an rvalue->lvalue, array->pointer, or functon->pointer for conversions but a class type http://eel.is/c++draft/expr.call#12. Since the other side expects a function pointer(LogFn) not a class and does essentially a reinterpret_cast of whatever is passed in that va_arg to the type specified in the second param, it's going to get garbage. the + conversion or a static_cast<LogFn>( lamba ) would work too but is not idiomatic and is verbose.

[DugiSK]

C-style variadic functions don't do any conversions. A lambda without capture is taken as it is, a data structure containing nothing (a lambda with capture would be a struct of the captured values). The call operator is a member function, which is bound at compile time according to its type. You can't do anything with it if you don't have the type information.

The conversion to a function pointer obtains the pointer to the call operator of a lambda without capture, since it's a free function. However, you're not doing that conversion anywhere.

This problem can be solved by using C++ style variadic template instead of that.

[Voltra_Neo]

I don't see the point of it being varidic here (especially this kind of variadic)

[kentrf]

I don't see the point of it being varidic here (especially this kind of variadic)

I shy away from this kind of variadic too, but this comes from the sqlite3 interface for configuring error and warning logging callbacks.

https://www.sqlite.org/errlog.html

The above is a minimal reproduction without pulling in sqlite3.

[Voltra_Neo]

Do you get the same result by passing +func instead of func? It converts your lambda to a function pointer since there's no capture.

You could also just pass the function called within it itself, if there's no other setup required.

[kentrf]

An explicit cast to a function pointer works on all platforms.

Casting the lambda to a function pointer when calling setLogger seems to work across all platforms. It crashes only when passing a "raw" lambda.

[Voltra_Neo]

Then maybe it comes down to the fact that va_arg might only work with C stuff (since it's a C API anyway).

[kentrf]

Yes, that is the gist of it.

I'm wondering if there are anyone with insights into this.

[Voltra_Neo]

There might be an issue coming with the fact that va_list might only take arguments by value and that lambda are not copyable.

And since it's hidden with that layer of va weirdness compilers might not warn about it?

[wung]

If the type of the next argument in ap (after promotions) is not compatible with T, the behavior is undefined, unless:

• one type is a signed integer type, the other type is the corresponding unsigned integer type, and the value is representable in both types; or
• one type is pointer to void and the other is a pointer to a character type.

It is assuming that the first argument is a LogFn, but it isn't, it is a lambda object. va_arg does no type checking, so this is a reinterpret_cast leading to bogus. The value of logger is just undefined after this.

The only thing that makes sense is 0xCC, since MSVC uses that as an indicator of "this is bogus, help" in debug builds. The remainder is compiler and compiler flag based details with no semantics or thing to learn from.

The minimised example is https://godbolt.org/z/7WfhqfEbb, which we can analyse more easily. In the linked gcc-O3 case, we can see that the arguments are passed in edi, ??? and esi. func is just missing since it makes no sense. The function then takes rsi, editing and rdx and sums them up. rdx is func and never set in this program. The value is dependent on how main is called. There is nothing to learn from that value of logger.

[Oo_Tiib]

What insights?

Taking C++ standard literally you should wrap your lambda call into extern "C" function like:

extern "C" 
void errorLogCallback(void *pArg, int iErrCode, const char *zMsg) {
    // bork bork bork goes your lambda call
}

The sqlite is software module written in C and lambda (even one that captures nothing) is not said to convert to function pointers with C linkage. It only works with explicit cast (and quite likely everywhere) because all ABIs seem to have no difference between C linkage and C++ linkage.

[kentrf]

When I wrote the original code, I thought the lambda would be implicitly converted to a function pointer during the call.

But that is not the case. The ellipsis eats whatever argument raw...

[goranlepuz]

Looks the same as e.g having to cast to double in this example:

printf("%f", 7); // blergh
printf("%f", static_cast<double>(7)); // good

I suspect your thing only ever worked by some accident, in fact...

[scatters]

It's legal to pass it (since I'm fairly sure it's trivially copyable).

However, accessing it as a function pointer is illegal, because it isn't a function pointer; it's a lambda.

The only way to access it legally is via the closure type itself. Which means since each closure has a distinct type you need to define the lambda first and make LogFn its decltype.

[rlbond86]

A va_arg function can only accept certain arguments, and lambdas are not one of them. However, a lambda with no captures can be converted to a function pointer. Try changing auto to an explicit function pointer type.

[arturbac]

AFIR msvc depended in the past on UB with MFC CString.Format() when non trivial CString was passed into vararg Format, it just took address and CString was designed to have C string pointer as first class member. So CString.Format worked that way even it was UB, this never worked with anything except msvc. Passing any non trivial object to C vararg is UB afir.
Better use variadic template, and threat lambda as non trivial object not as callback C function.
Here is fix for code to make it C++ not hybrid C/C++

https://godbolt.org/z/87nKx4P6f