stylo: Gecko_GetBaseSize writing to nsFont.size

NEW
Assigned to

Status

()

Core
CSS Parsing and Computation
P1
normal
3 months ago
3 months ago

People

(Reporter: sfink, Assigned: sfink, NeedInfo)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

3 months ago
  [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?
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)
Manish, what's the next step here. Is the analysis wrong?
Flags: needinfo?(manishearth)
Priority: -- → P1
Yes, it is, I chatted with sfink about this, he's looking into it. I don't know how to fix the analysis here.
Flags: needinfo?(manishearth)
Ok, over to Steve.
Assignee: nobody → sphink
Flags: needinfo?(sphink)
You need to log in before you can comment on or make changes to this bug.