non-POD types passed to .../va_start is UB

[gdt]

In capi/geos_ts_c.cpp, the NOTICE and ERROR functions are variadic and take a std::string as the first argument. They then call va_start with this non-POD type. This lands us in "conditionally defined" behavior (5.2.2 clause 7), which given multiple possible compilers is really "undefined behavior". Some links which help explain why this is a painful situation.

• ​https://reviews.llvm.org/D29256
• ​https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30457

I have attached a patch which avoids the issue, essentially by hoisting the c_str() before the call (and making it implicit). With this patch and 3.8.1, tests pass on NetBSD 8 amd64 and macOS 10.13 amd64.

[gdt]

This might as well be fixed in 3.8.2 as well as master; picking 3.8.1 as meaning 3.8.2.

[pramsey]

Running tests at ​https://github.com/libgeos/geos/pull/308

[pramsey]

Huh, I actually have a unit test failure on OSX, with this patch still.

Darwin Kernel Version 19.4.0
MacOS 10.15

CI seems has come up green though...

[Paul Ramsey <pramsey@…>]

In e6f315d/git:

Remove undefined behaviour in CPI, closes #1021