Fix a potential overflow in core::str::Searcher::new

[tbu-]

The overflow is mitigated by checking a sufficient condition for the less
relation.

Given the term A - B < C (A, B and C fixed size unsigned integers) one
can check whether it holds, by evaluating A < C || A - B < C.

[alexcrichton]

What overflow is this fixing? When needle.len() is more than 20 below uint::MAX?

[alexcrichton]

That is, if needle.len() is more than 20 below uint::MAX, you have a string which has consumed all but 20 bytes of the address space, and you've likely got bigger problems than searching for a substring inside of it.

[thestinger]

It's impossible for the length to be 20 below uint::MAX so perhaps a comment to that effect is enough.

[tbu-]

Is it theoretically impossible for the length to be 20 below uint::MAX? If so, a comment should suffice.

If it's theoretically possible then maybe some guideline on "what can we expect from array lengths" would be good so it's consistent over all Rust code. (Maybe: All arrays of objects with size 1 can be assumed to have a maximum int::MAX elements, and int can be assumed to be at least i8).

[thestinger]

It's impossible because uint::MAX is defined to be the maximum possible number of addressable bytes and there are various reasons why 20 bytes will certainly be used. One reason is that the main stack size must be at least 4k. I think we should clearly define some assumptions about int and uint, such as whether int can address the whole address space with the positive range. I don't have an opinion on the best choices right now.

On modern operating systems, the kernel gets half of the address space and userspace gets the other half, so saying int can address the whole range won't lose anything. On bare hardware without that distinction, the guarantee would make 1 bit of the address space unusable. It could just be considered a portability issue as long as there's documentation to that effect somewhere - new ports may need to add a check, but no existing one needs it.

[tbu-]

If #16715 is fixed, this can be replaced by a comment instead of the check.

[alexcrichton]

As-is, I believe that this fix is not necessary, @tbu- can you update this PR to have a comment instead?

[bluss]

@alexcrichton The PR author is writing correct code and I think it's something we should observe better throughout libcore/libstd (I've hunted such problems before). Being correct one time too many would be a good start.

[alexcrichton]

Closing due to inactivity, but feel free to reopen with my comment addressed!