1838739 - SetAsGPUOutOfMemoryError() called but its OwningNonNull is left uninitialized

Status: RESOLVED FIXED

(Core :: Graphics: WebGPU, defect, P1)

Comment 1

[Kelsey Gilbert [:jgilbert]]

From bug 1838703, here's the root cause analysis for the regression surfaced (though not introduced) by bug 1837557.

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!

Comment 2

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1838739 - Initialize result of SetAsGPUOutOfMemoryError.

Comment 3

[Kelsey Gilbert [:jgilbert]]

The patch here is required for the patch in bug 1838703 to work, since bug 1838703 catches this issue.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8599db8132f9
Initialize result of SetAsGPUOutOfMemoryError. r=webgpu-reviewers,nical

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1837557

Comment 6

[Cosmin Sabou [:CosminS]]

Backed out for causing build bustages on OutOfMemoryError.h.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=8599db8132f994309afbe27805208fb147fbcf0c&selectedTaskRun=fYFkA8OaROKPW40Gg575sQ.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=419505508&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/fa529bbf11c86465c2e1ecfd68b07ae27cbfcb87

Comment 7

[Erich Gubler [:ErichDonGubler]]

I believe that :jgilbert has addressed the cause of the backout in D181161 already (added explicit to the OutOfMemoryError constructor), so I'm gonna try to land this.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ec4b06c796a
Initialize result of SetAsGPUOutOfMemoryError. r=webgpu-reviewers,nical

Comment 9

[Serban Stanca [:SerbanS]]

Backed out for causing build bustages in OutOfMemoryError.h

• Backout link
• Push with failures
• Failure Log
• Failure line: builds/worker/workspace/obj-build/dist/include/mozilla/webgpu/OutOfMemoryError.h:24:12: error: missing return type for function 'sOutOfMemoryError'; did you mean the constructor name 'OutOfMemoryError'?

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f0ecff11fae
Initialize result of SetAsGPUOutOfMemoryError. r=webgpu-reviewers,nical

Comment 11

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/1f0ecff11fae

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1837557