Open Bug 1578953 Opened 6 years ago Updated 3 years ago

Conditional jump or move depends on uninitialised value(s): real_drop_in_place<core::option::Option<style::values::specified::color::Color>> (ptr.rs:195)

Categories

(Core :: CSS Parsing and Computation, defect, P3)

x86_64
Linux
defect

Tracking

()

People

(Reporter: ishikawa, Unassigned)

Details

This is with M-C and C-C updated a couple of days ago.
I am using linux amd64, but I think the problem is universal across platforms.
(Well, I am using gcc-8 for local compilation if it matters.)
Found by valgrind while I was testing locally build FULL DEBUG vesion of TB by |make mozmill test suite|.

Excerpt from valgrind log:
I am not sure if the line number 195 as in ptr.rs:195 on the topmost line of stacktrace is correct or not. (I found ptr.rs, but it only contains the following lines.(???)
//! Vector of pointers

#[macro_use]
mod gather_scatter;

==31965== Thread 1:
==31965== Conditional jump or move depends on uninitialised value(s)
==31965== at 0xF70B028: real_drop_in_place<core::option::Option<style::values::specified::color::Color>> (ptr.rs:195)
==31965== by 0xF70B028: style::values::specified::effects::<impl style::parser::Parse for style::values::generics::effects::GenericBoxShadow<core::option::Option<style::values::specified::color::Color>,style::values::specified::length::Length,core::option::Option<style::values::generics::NonNegative<style::values::specified::length::Length>>,core::option::Option<style::values::specified::length::Length>>>::parse (effects.rs:192)
==31965== by 0xF6AED41: parse (effects.rs:201)
==31965== by 0xF6AED41: {{closure}} (effects.rs:331)
==31965== by 0xF6AED41: call_once<(&mut cssparser::parser::Parser),closure> (function.rs:279)
==31965== by 0xF6AED41: parse_entirely<&mut closure,style::values::generics::effects::GenericBoxShadow<core::option::Option<style::values::specified::color::Color>, style::values::specified::length::Length, core::option::Option<style::values::generics::NonNegative<style::values::specified::length::Length>>, core::option::Option<style::values::specified::length::Length>>,style_traits::StyleParseErrorKind> (parser.rs:634)
==31965== by 0xF6AED41: cssparser::parser::parse_until_before (parser.rs:975)
==31965== by 0xF56A5D2: parse_until_before<&mut closure,style::values::generics::effects::GenericBoxShadow<core::option::Option<style::values::specified::color::Color>, style::values::specified::length::Length, core::option::Option<style::values::generics::NonNegative<style::values::specified::length::Length>>, core::option::Option<style::values::specified::length::Length>>,style_traits::StyleParseErrorKind> (parser.rs:709)
==31965== by 0xF56A5D2: parse_comma_separated<closure,style::values::generics::effects::GenericBoxShadow<core::option::Option<style::values::specified::color::Color>, style::values::specified::length::Length, core::option::Option<style::values::generics::NonNegative<style::values::specified::length::Length>>, core::option::Option<style::values::specified::length::Length>>,style_traits::StyleParseErrorKind> (parser.rs:664)
==31965== by 0xF56A5D2: parse<closure,style::values::generics::effects::GenericBoxShadow<core::option::Option<style::values::specified::color::Color>, style::values::specified::length::Length, core::option::Option<style::values::generics::NonNegative<style::values::specified::length::Length>>, core::option::Option<style::values::specified::length::Length>>,style_traits::StyleParseErrorKind> (values.rs:303)
==31965== by 0xF56A5D2: style::properties::longhands::box_shadow::parse (effects.rs:330)
==31965== by 0xF58D7B1: parse_declared (effects.rs:398)
==31965== by 0xF58D7B1: style::properties::LonghandId::parse_value (properties.rs:44180)
==31965== by 0xF5A6245: {{closure}} (properties.rs:53587)
==31965== by 0xF5A6245: parse_entirely<closure,style::properties::PropertyDeclaration,style_traits::StyleParseErrorKind> (parser.rs:634)
==31965== by 0xF5A6245: {{closure}} (properties.rs:53587)
==31965== by 0xF5A6245: or_else<style::properties::PropertyDeclaration,(),cssparser::parser::ParseError<style_traits::StyleParseErrorKind>,closure> (result.rs:709)
==31965== by 0xF5A6245: style::properties::PropertyDeclaration::parse_into (properties.rs:53581)
==31965== by 0xF6B81C3: {{closure}} (declaration_block.rs:1313)
==31965== by 0xF6B81C3: parse_entirely<closure,(),style_traits::StyleParseErrorKind> (parser.rs:634)
==31965== by 0xF6B81C3: cssparser::parser::parse_until_before (parser.rs:975)
==31965== by 0xF7213A2: parse_until_before<closure,(),style_traits::StyleParseErrorKind> (parser.rs:709)
==31965== by 0xF7213A2: <style::properties::declaration_block::PropertyDeclarationParser as cssparser::rules_and_declarations::DeclarationParser>::parse_value (declaration_block.rs:1312)
==31965== by 0xF695F9A: {{closure}}<style::properties::declaration_block::Importance,style::properties::declaration_block::PropertyDeclarationParser,style_traits::StyleParseErrorKind> (rules_and_declarations.rs:290)
==31965== by 0xF695F9A: parse_entirely<closure,style::properties::declaration_block::Importance,style_traits::StyleParseErrorKind> (parser.rs:634)
==31965== by 0xF695F9A: parse_until_before<closure,style::properties::declaration_block::Importance,style_traits::StyleParseErrorKind> (parser.rs:975)
==31965== by 0xF695F9A: parse_until_before<closure,style::properties::declaration_block::Importance,style_traits::StyleParseErrorKind> (parser.rs:709)
==31965== by 0xF695F9A: cssparser::parser::parse_until_after (parser.rs:1004)
==31965== by 0xF53F9E8: <cssparser::rules_and_declarations::DeclarationListParser<P> as core::iter::traits::iterator::Iterator>::next (rules_and_declarations.rs:285)
==31965== by 0xF72268A: style::properties::declaration_block::parse_property_declaration_list (declaration_block.rs:1423)
==31965== by 0xF709A47: <style::stylesheets::rule_parser::NestedRuleParser as cssparser::rules_and_declarations::QualifiedRuleParser>::parse_block (rule_parser.rs:599)
==31965== by 0xF69C722: parse_block (rule_parser.rs:322)
==31965== by 0xF69C722: {{closure}}<style::stylesheets::rule_parser::TopLevelRuleParser,style_traits::StyleParseErrorKind> (rules_and_declarations.rs:546)
==31965== by 0xF69C722: parse_entirely<closure,style::stylesheets::CssRule,style_traits::StyleParseErrorKind> (parser.rs:634)
==31965== by 0xF69C722: cssparser::parser::parse_nested_block (parser.rs:1048)
==31965== by 0xF5545AF: cssparser::rules_and_declarations::parse_qualified_rule (rules_and_declarations.rs:545)
==31965== by 0xF53B3BC: <cssparser::rules_and_declarations::RuleListParser<P> as core::iter::traits::iterator::Iterator>::next (rules_and_declarations.rs:415)
==31965== by 0xF5F4EF2: style::stylesheets::stylesheet::Stylesheet::parse_rules (stylesheet.rs:418)
==31965== by 0xF5F3B7A: style::stylesheets::stylesheet::StylesheetContents::from_str (stylesheet.rs:86)
==31965== by 0xF330CCE: Servo_StyleSheet_FromUTF8Bytes (glue.rs:1371)
==31965== by 0xCBB8E52: mozilla::StyleSheet::ParseSheet(mozilla::css::Loader&, nsTSubstring<char> const&, mozilla::css::SheetLoadData&) (StyleSheet.cpp:964)
==31965== by 0xCBA078B: mozilla::css::Loader::ParseSheet(nsTSubstring<char> const&, mozilla::css::SheetLoadData&, mozilla::css::Loader::AllowAsyncParse) (Loader.cpp:1661)
==31965== by 0xCBAE07A: mozilla::css::StreamLoader::OnStopRequest(nsIRequest*, nsresult) (StreamLoader.cpp:109)
==31965== by 0x92C00C9: nsBaseChannel::OnStopRequest(nsIRequest*, nsresult) (nsBaseChannel.cpp:815)
==31965== by 0x92D05C4: nsInputStreamPump::OnStateStop() (nsInputStreamPump.cpp:655)
==31965== by 0x92E2EEE: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (nsInputStreamPump.cpp:403)
==31965== by 0x91388F4: nsInputStreamReadyEvent::Run() (nsStreamUtils.cpp:91)
==31965== by 0x91B273B: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:1225)
==31965== by 0x91A40DD: NS_ProcessNextEvent(nsIThread*, bool) (nsThreadUtils.cpp:486)
==31965== by 0x984B837: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (MessagePump.cpp:88)
==31965== by 0x97F745C: MessageLoop::RunInternal() (message_loop.cc:315)
==31965== by 0x97F74BA: MessageLoop::RunHandler() (message_loop.cc:308)
==31965== by 0x97F77F9: MessageLoop::Run() (message_loop.cc:290)
==31965== by 0xC98D644: nsBaseAppShell::Run() (nsBaseAppShell.cpp:137)
==31965== by 0xDE30B8F: nsAppStartup::Run() (nsAppStartup.cpp:276)
==31965== by 0xDF5E0BC: XREMain::XRE_mainRun() (nsAppRunner.cpp:4573)
==31965== by 0xDF5F558: XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (nsAppRunner.cpp:4711)
==31965== by 0xDF5FC56: XRE_main(int, char**, mozilla::BootstrapConfig const&) (nsAppRunner.cpp:4792)
==31965== by 0xDF61B12: mozilla::BootstrapImpl::XRE_main(int, char**, mozilla::BootstrapConfig const&) (Bootstrap.cpp:45)
==31965== by 0x11261A: do_main(int, char**, char**) (nsMailApp.cpp:210)
==31965== by 0x1126C8: main (nsMailApp.cpp:285)
==31965==

TIA

Thanks, there's a bunch of these Valgrind warnings that result from Rust code. I don't think we have a better solution that to just add suppressions for them.

Priority: -- → P3

(In reply to Cameron McCormack (:heycam) from comment #1)

Thanks, there's a bunch of these Valgrind warnings that result from Rust code. I don't think we have a better solution that to just add suppressions for them.

Thank you for your comment.

So are we sure we can safely ignore this?

I looked at a few other bugzilla entries, and a comment in one of the entries mentioned maybe llvm/rust coptimizes code that fools valgrind (?!).

Since I am passing "-g" for debug and do not specify optimization, I am a bit surprised.
I think rust compiler should not try to optimize when it is not instructed to do so.
Without such optimization, MAYBE this and other issues reported by valgrind may not happen. (I have not looked into how rustc is invoked. |mach build| does not seem to report the details of rustc command line by default.

Anyway, over the past several years, I could find errors of TB thanks to valgrind warnings and the number of valgrind warnings have diminished almost zero for TB over the years, so I wish llvm/rustc can work with valgrind nicely.
One bug was deemed even security issue and I got bounty.
(Lately I realize pytho3.7 interpreter which drives |make mozmill| test suite has a use-after-free issues thanks to valgrind !)

So I say valgrind is useful.

Some people may think JavaScript/Rustc may be free from such uninitialized value errors.
This is NOT SO (!) in mozilla code.
Mozilla code uses XCOM to pass data between C++/JavaScript/Rust. Possibly uninitialized data from C++ side may be passed to Rustc where unintended operation may happen.
I have seen JavaScript code triggered uninitialized data access because of this XPCOM passing uninitialized data to JavaScript from C++.
Debugging it was a tricky, but I managed to find where the data was not uninitialized. (Surprisingly, many class variables failed to get initialized upon object creation.)
So a confidence that in a self-contained rustc code may not cause such uninitialized value access at all is not helpful in checking mozilla FF or TB code that uses XCOM to run C++/JavaScript/Rustc codes together.

I will add a few suppression in the meantime, but I do hope rustc experts or build configuration experts take a look and disable optimization of rustc unless the user explicitly requests it. (I m assuming that this will help reduce the false positives. But if not, we have a big debug/test issue for mozilla code. In the long run, the testing / debugging time consumes developer time longest. We should make sure tools work with debugging framework nicely.)

TIA

So are we sure we can safely ignore this?

No, until we look into the assembly it generates. But given this one involves enum, it's very likely to be the case from the llvm optimization which can swap order of conditions which is safe if you look from a higher level.

If you can reproduce this, you can try disasm the function and see whether it's the case. It would be alarming if it's not.

It may be the case that it's the GCC bit which matters, see bug 1600735 for fun.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

It may be the case that it's the GCC bit which matters, see bug 1600735 for fun.

Oh, what a read. I hope the LLVM folks will be quick to fix the subtle issue...
In the meantime, I see that I am using GCC-9.

Right now, the local build does not compile due to bug 1603982
https://bugzilla.mozilla.org/show_bug.cgi?id=1603982
and I will report any other strange issues related to valgrind/gcc worth mentioning here when they come up...

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.