Closed
Bug 1838703
Opened 1 year ago
Closed 1 year ago
Bindings::SetAsFoo() methods that return OwningNonNull/NonNull should be [[nodiscard]]
Categories
(Core :: DOM: Bindings (WebIDL), defect, P1)
Core
DOM: Bindings (WebIDL)
Tracking
()
RESOLVED
FIXED
116 Branch
Tracking | Status | |
---|---|---|
firefox116 | --- | fixed |
People
(Reporter: jgilbert, Assigned: jgilbert)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
class OwningGPUOutOfMemoryErrorOrGPUValidationError : public AllOwningUnionBase
[...]
union Value
{
UnionMember<OwningNonNull<mozilla::webgpu::OutOfMemoryError> > mGPUOutOfMemoryError;
UnionMember<OwningNonNull<mozilla::webgpu::ValidationError> > mGPUValidationError;
};
TypeOrUninit mType;
Value mValue;
OwningNonNull<mozilla::webgpu::OutOfMemoryError>&
OwningGPUOutOfMemoryErrorOrGPUValidationError::SetAsGPUOutOfMemoryError()
{
if (mType == eGPUOutOfMemoryError) {
return mValue.mGPUOutOfMemoryError.Value();
}
Uninit();
mType = eGPUOutOfMemoryError;
return mValue.mGPUOutOfMemoryError.SetValue();
}
class UnionMember {
AlignedStorage2<T> mStorage;
[...]
template <typename... Args>
T& SetValue(Args&&... args) {
new (mStorage.addr()) T(std::forward<Args>(args)...);
return *mStorage.addr();
}
template <class T>
class MOZ_IS_SMARTPTR_TO_REFCOUNTED OwningNonNull {
public:
using element_type = T;
OwningNonNull() = default;
The problem is that SetValue() in-place constructs the union variant, and in this case the union variant isn't OutOfMemory (which has a deleted ctor), it's OwningNonNull<OutOfMemory>
, which does have a super dangerous default constructor!
Assignee | ||
Comment 1•1 year ago
|
||
The bug in question that the fix for bug 1837557 started hitting on CI is:
dom::OwningGPUOutOfMemoryErrorOrGPUValidationError error;
[...]
case PopErrorScopeResultType::OutOfMemory:
error.SetAsGPUOutOfMemoryError();
break;
Assignee | ||
Comment 2•1 year ago
|
||
Updated•1 year ago
|
Blocks: webgpu-v1-cts-windows
Comment 3•1 year ago
|
||
Marking as blocking webgpu-v1-cts-windows
, since the scope of comment 2 on bug 1838694 has moved to this bug at :jgilbert's initiative.
Assignee | ||
Updated•1 year ago
|
Pushed by jgilbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5cd8f2638cc5 [dom/bindings] Mark SetAsFoo() as nodiscard. r=peterv
Comment 5•1 year ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 1 year ago
status-firefox116:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 116 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•