Re: [libmicrohttpd] Three runtime errors in tests with UBSAN

[Tim Ruehsen]

Am Freitag, den 02.03.2018, 22:07 +0100 schrieb Christian Grothoff:
> On 02/27/2018 10:39 AM, Tim Rühsen wrote:
> > $ CC=gcc CFLAGS="-O0 -g -ggdb3 -fno-omit-frame-pointer
> > -fsanitize=undefined" ./configure
> > 
> > $ make clean
> > $ make check
> > $ grep runtime src/*/*.log
> > 
> > src/microhttpd/test_upgrade.log:test_upgrade.c:1075:32: runtime
> > error:
> > member access within misaligned address 0x560144586014 for type
> > 'const
> > union MHD_DaemonInfo', which requires 8 byte alignment
> > 
> > src/microhttpd/test_upgrade_tls.log:test_upgrade.c:1075:32: runtime
> > error: member access within misaligned address 0x55cea9a95014 for
> > type
> > 'const union MHD_DaemonInfo', which requires 8 byte alignment
> 
> I'm not sure I agree with these two, as while we return a 'union'
> with
> an 8-byte alignment, the only valid *field* we return is 16-bit, and
> the
> union overall is read-only.  So reading the 16-bit field should be
> perfectly safe from an alignment perspective (it is 16-bit aligned),
> and
> everything else is a really bad idea (TM) anyway, as it would read
> garbage.

I agree with that analysis.
But the sanitizer's instrumentation code just sees the issue from a
peephole perspective and so doesn't know what the callers are doing
with the returned union pointer. So it correctly shouts out.


> We could theoretically fix this, but to avoid ABI breakage this would
> require using more memory (effectively reserving a fresh union for
> each
> possible return case) and I do not see how this would gain anything
> for
> anyone except making the sanitizer happy.
> 
> A better solution is of course the upcoming microhttpd2-API, which
> avoids this issue entirely (by not returning a union).

That is of course the best solution.

> For now, I've just hacked the testcase to memcpy() the port, which
> doesn't actually improve the code but eliminates the warning. 8-)

You could also just silence the sanitizer, e.g. adding a function
attribute like (didn't test if "address" is the correct one here):

#ifdef __clang__
__attribute__((no_sanitize("address")))
#endif


> > src/testcurl/test_put_chunked.log:test_put_chunked.c:107:16:
> > runtime
> > error: null pointer passed as argument 1, which is declared to
> > never be null
> 
> Fixed.

Thanks !

Regards, Tim