Avoid 1 << 31 (UB) in calculation of SEQUENCE_LOCKTIME_DISABLE_FLAG #14513

Avoid 1 << 31 (UB) in calculation of SEQUENCE_LOCKTIME_DISABLE_FLAG.

Context: #14510 (comment)

ACK bc60c61.

Interestingly, this becomes well defined in C++14: https://en.cppreference.com/w/cpp/language/operator_arithmetic#Bitwise_shift_operators

AHHHHH not another one

@laanwj Having an UBSAN build running the functional tests would help us guard against the introduction of new UB:s in the code base.

I've said it before but I find it troubling that we don't use all relevant sanitizers (ASAN, UBSAN, etc.) in Travis. I hope to be able to help change that: please review or concept ACK the PR #14252 which introduces an UBSAN Travis build running the functional tests.

Regarding UB:s already in the code base – I'll keep on digging: see https://twitter.com/practicalswift/status/1051524615204470784 :-) (Context: UB is one of Professor Regehr's areas of research – see his excellent papers and blog posts about UB)

but this is a constant ! shouldn't it be possible to detect this statically?

@laanwj Yes, absolutely. Dynamic analysis and static analysis are just two different approaches to attacking the same problem: both should be used. (Just to be clear: obviously this specific case cannot be detected by dynamic analysis.)

I was talking about dynamic analysis here since I believe the sanitizers would likely give us the most bang for the buck (especially ASAN and UBSAN) in terms of both precision and recall (for catching memory corruption bugs such as use-after-free:s, UB:s in general, etc.). So that's a good start.

But as you know I have a lot of love also for static analysis (various forms of linters, etc.) :-) Dynamic or static – I love 'em all!

ACK bc60c61

ACK