1613229 - Conditional jump or move depends on uninitialised value [@ style::properties::shorthands::font_variant::parse_value]

Status: NEW

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

Description

[Tyson Smith [:tsmith]]

Report is from m-c 20200203-9f8766e42efe

Conditional jump or move depends on uninitialised value(s)
   at 0x150A8A48: style::properties::shorthands::font_variant::parse_value (mod.rs:175)
   by 0x1503066A: style::properties::PropertyDeclaration::parse_into (font.rs:993)
   by 0x150F0E5B: style::properties::declaration_block::parse_property_declaration_list (declaration_block.rs:1342)
   by 0x150EF9D6: <style::stylesheets::rule_parser::NestedRuleParser as cssparser::rules_and_declarations::QualifiedRuleParser>::parse_block (rule_parser.rs:599)
   by 0x150EF512: cssparser::rules_and_declarations::parse_qualified_rule (rule_parser.rs:322)
   by 0x15109E43: style::stylesheets::stylesheet::StylesheetContents::from_str (rules_and_declarations.rs:402)
   by 0x14C97F56: Servo_StyleSheet_FromUTF8Bytes (glue.rs:1548)
   by 0x12EDAEAE: mozilla::StyleSheet::ParseSheetSync(mozilla::css::Loader*, nsTSubstring<char> const&, mozilla::css::SheetLoadData*, unsigned int, mozilla::css::LoaderReusableStyleSheets*) (layout/style/StyleSheet.cpp:1103)
   by 0x12EC79A4: mozilla::css::Loader::ParseSheet(nsTSubstring<char> const&, mozilla::css::SheetLoadData&, mozilla::css::Loader::AllowAsyncParse) (layout/style/Loader.cpp:1664)
   by 0x12EC86BB: mozilla::css::Loader::LoadInlineStyle(nsIStyleSheetLinkingElement::SheetInfo const&, nsTSubstring<char16_t> const&, unsigned int, nsICSSLoaderObserver*) (layout/style/Loader.cpp:1969)
   by 0x117B6E03: nsStyleLinkElement::DoUpdateStyleSheet(mozilla::dom::Document*, mozilla::dom::ShadowRoot*, nsICSSLoaderObserver*, nsIStyleSheetLinkingElement::ForceUpdate) (dom/base/nsStyleLinkElement.cpp:337)
   by 0x11116DBC: nsHtml5DocumentBuilder::UpdateStyleSheet(nsIContent*) (parser/html/nsHtml5DocumentBuilder.cpp:69)
 Uninitialised value was created by a stack allocation
   at 0x150A7AFD: style::properties::shorthands::font_variant::parse_value (font.rs:1028)

Comment 1

[Tyson Smith [:tsmith]]

Julian, I'm not sure if this is a false positive or not. I am using Valgrind built from master commit 82da9c1006c29a04878ef7a5c9c71b397155c606. I do have a test case that is unreduced that does consistently reproduce this issue.

Comment 2

[Daniel Holbert [:dholbert]]

Calling this P2 for the uninitialized-memory-read implications, if it's not a false positive.

Comment 3

[Daniel Holbert [:dholbert]]

emilio, perhaps you could take a look -- does the backtrace here make any sense to you, and do you see a possibility of an uninitialized memory dependency in the logic? (from code inspection)

Comment 4

[Julian Seward [:jseward]]

Tyson, Emilio, sorry to be slow on this. The fact that this is Rust increases
the probability that this is a false positive, unless there is some unsafeness
in there somewhere. On the other hand, Tyson is using Valgrind trunk sources,
which contain the latest anti-false-positive work. So I'm not sure what to
make of it.

It would help to have a disassembly of the instructions from say 10 before
the point of complaint (0x150A8A48) to 10 afterwards. Do either of you
know how to drive objdump -d? In short, first rerun Valgrind with
--demangle=no --sym-offsets=yes. That will tell you the point-of-complaint
as a symbol name + offset. Then objdump -d the relevant object, probably
libxul.so, find the symbol, then the instruction. The lowest 12 bits of the
instruction address will be as shown in the backtrace. Given the size of
libxul.so, this last step can be difficult.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I don't think there's any unsafe involved here... Tyson, can you do ^ given you already have the valgrind-trunk setup? If not, ni? me back and I'll try.

Comment 6

[Tyson Smith [:tsmith]]

Attachment: log_valgrind.24893.25081.txt

I was able to get a log that contains the point-of-complaint from a build from TC[1]. This info is all that is needed to perform the objdump step if I understand correctly (which I was unable to do). I'd be happy to do that as well if someone can provide steps.

[1] https://firefox-ci-tc.services.mozilla.com/tasks/index/gecko.v2.mozilla-central.pushdate.2020.02.24.20200224162717.firefox/linux64-valgrind-opt