1809880 - Invalid downcast in ~ScriptLoadRequest

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[lukas.bernhard]

Steps to reproduce:

When building with -fsanitize=cfi-derived-cast, the sanitizer detects some incorrect casting in ScriptLoadRequest::~ScriptLoadRequest. The destructor calls IsMarkedForBytecodeEncoding, which in turn downcast this to the derived class ModuleLoadRequest. However, ModuleLoadRequest's destructor already ran.

#0  0x00007efc02df78d5 in JS::loader::ScriptLoadRequest::AsModuleRequest (this=0x7efbe64d3100) at js/loader/ScriptLoadRequest.cpp:154
#1  0x00007efc02df74f7 in JS::loader::ScriptLoadRequest::IsMarkedForBytecodeEncoding (this=0x7efbe64d3100) at js/loader/ScriptLoadRequest.cpp:191
#2  0x00007efc02df73d7 in JS::loader::ScriptLoadRequest::~ScriptLoadRequest (this=0x7efbe64d3100) at js/loader/ScriptLoadRequest.cpp:99
#3  0x00007efc02df90b0 in JS::loader::ModuleLoadRequest::~ModuleLoadRequest (this=0x7efbe64d3100) at js/loader/ModuleLoadRequest.h:39
#4  0x00007efc02df90d9 in JS::loader::ModuleLoadRequest::~ModuleLoadRequest (this=0x7efbe64d3100) at js/loader/ModuleLoadRequest.h:39
#5  0x00007efc02df70dc in JS::loader::ScriptLoadRequest::DeleteCycleCollectable (this=0x7efbe64d3100) at js/loader/ScriptLoadRequest.cpp:57

Comment 1

[Emilio Cobos Álvarez [:emilio]]

I don't understand why that whole block is needed? It's effectively just nulling out a pointer, which will be nulled out as part of the destructor?

Comment 2

[Yulia Startsev [:yulia] OOO until July 2026]

This predates my work (I just moved it) so I don't have the historical context for this, but yeah that looks a bit weird. We drop everything on cycle collection regardless of if it was cached or not : https://searchfox.org/mozilla-central/rev/893a8f062ec6144c84403fbfb0a57234418b89cf/js/loader/ScriptLoadRequest.cpp#64

That can probably be replaced a default destructor.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1809880 - Avoid cast to derived type in ScriptLoadRequest base class destructor r?emilio

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f00420be3277
Avoid cast to derived type in ScriptLoadRequest base class destructor r=emilio

Comment 5

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/f00420be3277