Fix undefined in core-test and printf-test #1345
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
orivej
force-pushed
the
ubsan
branch
2 times, most recently
from
85385fa
to
ad0397e
Compare
vitaut
requested changes
Oct 7, 2019
include/fmt/core.h
Outdated
| @@ -1208,6 +1208,7 @@ template <typename Context> class basic_format_args { | |||
|
|
|||
| format_arg do_get(int index) const { | |||
| format_arg arg; | |||
| if (index < 0) return arg; | |||
There was a problem hiding this comment.
This should be fixed in fmt's printf implementation. It shouldn't be calling do_get with negative index.
There was a problem hiding this comment.
OK. I have also split the PR into 3 commits.
Fixes "reference binding to null pointer" in BufferTest.Ctor buffer.operator[] attempts to return a reference to `buffer.ptr_[0]` when `ptr_` in `mock_buffer<int> buffer` is null.
Fixes "signed integer overflow" in PrintfTest.Length
This occurs in `TestLength<long long>("ll")`, since its minimum value minus one
does not fit in long long.
Printf counts arguments from 1. Fixes "shift exponent -4 is negative" in PrintfTest.InvalidArgIndex. `do_get` is called with index -1 when `basic_printf_context.arg` is called with id 4294967295 when basic_printf_context::get_arg subtracts 1 from arg_index 0 in the format string "%0$d".
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The change in include/fmt/core.h fixes "shift exponent -4 is negative" in
PrintfTest.InvalidArgIndex
do_getis called with index -1 whenbasic_printf_context.argis calledwith id 4294967295 when basic_printf_context::get_arg subtracts 1 from
arg_index 0 in the format string "%0$d".
The change in test/core-test.cc fixes "reference binding to null pointer" in
BufferTest.Ctor
buffer.operator[] attempts to return a reference to
buffer.ptr_[0]whenptr_inmock_buffer<int> bufferis null.The change in test/printf-test.cc fixes "signed integer overflow" in
PrintfTest.Length
This occurs in
TestLength<long long>("ll"), since its minimum value minusone does not fit in long long.
This fixes all UBSAN reports except one in format-test (#1344).
I agree that my contributions are licensed under the {fmt} license, and agree to future changes to the licensing.