1356610 - Gecko_GetBaseSize writing to nsFont.size

Status: RESOLVED DUPLICATE of bug 1400442

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

Description

[Steve Fink [:sfink] [:s:]]

  [24.62s] #169 Analyzing Gecko_GetBaseSize ...
  Error: Field write nsFont.size
  Location: _ZN7mozilla18LangGroupFontPrefs10InitializeEP7nsIAtom$void mozilla::LangGroupFontPrefs::Initialize(nsIAtom*) @ https://searchfox.org/mozilla-central/source/layout/base/StaticPresData.cpp#199 ### SafeArguments: <this>
  Stack Trace:
  Gecko_GetBaseSize @ https://searchfox.org/mozilla-central/source/layout/style/ServoBindings.cpp#1635

<bholley>     Was this fixed by bug 1351200?

<sfink>    No. That change is in the build I used. But I would not be at all surprised if it turned this into a false positive. Looking...ok, if that's safe, I'll need you to explain why. It's looking up a member field of LangGroupFontPrefs, eg mDefaultVariableFont, and setting its size. The analysis knows that 'this' (the LangGroupFontPrefs) is safe, but I think this is going one dereference further.

<bholley> Hm, probably needs Manishearth to look at it. Can you file a bug and NI him?

Comment 1

[Manish Goregaokar [:manishearth]]

It's not going one dereference further, the size variable is inline in the nsFont (which in turn is inline in the LangGroupFontPrefs, which we create on the stack)

Comment 2

[Bobby Holley (:bholley)]

Manish, what's the next step here. Is the analysis wrong?

Comment 3

[Manish Goregaokar [:manishearth]]

Yes, it is, I chatted with sfink about this, he's looking into it. I don't know how to fix the analysis here.

Comment 4

[Bobby Holley (:bholley)]

Ok, over to Steve.