Fix use of uninitialized value in WastParser::ParseSimdV128Const. #2668
Conversation
src/wast-parser.cc
Outdated
| case 16: { | ||
| uint8_t value = 0; | ||
| result = ParseInt8(sv, &value, ParseIntType::SignedAndUnsigned); | ||
| if (Failed(result)) break; |
There was a problem hiding this comment.
But value is initialized above, no? What uninitialized value would be used in const_->set_v128_u8(lane, value); below?
I guess if you want to keep these checks then you should remove the = 0 initialization above? (i.e. we should either initialize it so its safe to use regardless of ParseInt result, or we should check the ParseInt result)
There was a problem hiding this comment.
Aha I got carried away with the code pattern. It's the float/double cases below that have the actual use of uninitialized value. I'll amend the commit.
There was a problem hiding this comment.
Thanks Sam; I revised the PR to only fix the use of an uninitialized uint32_t in the ParseF32 and ParseF64 functions, which is sufficient to fix the msan finding. PTAL
|
BTW, while changes like this are great, and very much appreciated, you should note that we (wabt contributors) don't currently spend any time trying to fix fuzzer reported bugs in wabt, because (a) we don't the the resources and (b) the inputs tend to come from trusted sources. Obviously its fantastic when somebody is able to take the time like this, but just a word of warning that if you start down this road it might be a very long one :) |
|
We use WABT in our test harness for running spec tests, but the sanitizers can't tell the difference between WABT and our own code that we actually do want to sanitize. I'll keep sending these as the sanitizers find them, unless it becomes untenable at which point we'll find another solution. Thanks for the prompt review! |
2 participants
Found this from msan. See b/461044733.