Re: [lmi] Unit tests hygiene

[Vadim Zeitlin]

On Thu, 9 Jun 2022 21:37:00 +0000 Greg Chicares <gchicares@sbcglobal.net> wrote:

GC> Here's an example that I think is UB, though UBSan doesn't flag it:
GC> 
GC>     // UBSAN doesn't complain about this; shouldn't it?
GC>     auto const b {db.query<oenum_alb_or_anb>(DB_ChildRiderMinAmt)};
GC>     LMI_TEST_EQUAL(25000, b);
GC> 
GC> Am I missing something, or is this truly a false negative?

 Sorry, I know I'm tired today (because I've already made a couple of bad
mistakes in the last hour), but what exactly is the problem here, i.e. why
should it be UB? AFAIK comparing an enum with an int is not UB, due to the
promotion rules. Am I missing some other reason here?

GC> Could you update xmlwrapp separately now please?

 I'll try to do it a.s.a.p. but right now I have a problem with the CI
builds, that I've already spent a couple of hours on, but without being
able to fix it or even to understand what's going on: the tests have
suddenly started failing there (which, confusingly, doesn't make the "make
unit_tests" target to fail, as I think I've already complained once or
twice in the past), so something must have changed with the environment,
but I can't reproduce the problem locally. Ideally I'd like to fix this
before submitting the xmlwrapp change so that it gets tested by the CI, but
if I still don't manage to do it tomorrow, I'll just test it manually here
and submit it nevertheless.

GC> I'd be happy enough if I could just enable ASan and UBSan,
GC> except that
GC>   -fsanitize=address,undefined \
GC> seems to imply
GC>   -fsanitize=address,undefined,leak \
GC> unless I failed to 'make clean' at some point where it was necessary.

 For some reason I've never used ASAN and UBSAN simultaneously, I think
that this might have been unsupported in their initial versions and while I
was vaguely aware that this has changed since then, I kept my (bad) old
habits. But I also always thought that LSAN ("leak sanitizer") was just a
part of ASAN, isn't this the case?

 I'm quite certain that whether it is or not, LSAN can be disabled by
setting ASAN_OPTIONS='detect_leaks=0' during execution, so perhaps this
could be used as a stopgap solution.

GC> BTW: reading the gcc documentation gave me the impression that this:
GC>   -fsanitize=address,undefined,leak
GC> doesn't enable all the checks we could have, and maybe this:
GC>   
-fsanitize=address,undefined,leak,pointer-compare,pointer-subtract,float-divide-by-zero,float-cast-overflow
GC> would be better. Do you happen to know whether that's correct?

 No, sorry, I had no idea about all these options and naively thought that
{A+UB}SAN enabled all the checks. Looking at them now, it seems that some
of them can slow down things a lot during execution, which is the reason
they're not enabled by default. And, also, some of them still require
setting ASAN_OPTIONS too, e.g. "detect_invalid_pointer_pairs=2" must be
used for pointer-{compare,subtract} to really have effect. Finally, an
article[1] I saw also recommended to use _GLIBCXX_SANITIZE_VECTOR to detect
memory issues when using memory allocated but not supposed to be used by
std::vector, which seems like a very useful thing to do.

 And another article[2] also mentions "bounds-strict" as not being enabled
by default in UBSAN as of gcc 11, as well as a few other settings to put
into ASAN_OPTIONS, which might be worth exploring.
 
 Thanks again for looking at this,
VZ

[1]: 
https://kristerw.blogspot.com/2018/06/useful-gcc-address-sanitizer-checks-not.html
[2]: 
https://airbus-seclab.github.io/c-compiler-security/gcc_compilation.html#runtime-sanitizers

pgplb0m6AeCiS.pgp
Description: PGP signature