1711811 - [gcc 11] error: 'this' pointer is null [-Werror=nonnull]

Status: RESOLVED FIXED

(Firefox :: Disability Access, defect)

Description

[Mike Hommey [:glandium]]

Building with GCC 11 fails with:

[task 2021-05-19T00:00:35.298Z] 00:00:35     INFO -  In file included from Unified_cpp_accessible_xpcom0.cpp:65:
[task 2021-05-19T00:00:35.299Z] 00:00:35     INFO -  /builds/worker/checkouts/gecko/accessible/xpcom/xpcAccessibleHyperText.cpp: In member function 'virtual nsresult mozilla::a11y::xpcAccessibleHyperText::GetTextAttributes(bool, int32_t, int32_t*, int32_t*, nsIPersistentProperties**)':
[task 2021-05-19T00:00:35.299Z] 00:00:35    ERROR -  /builds/worker/checkouts/gecko/accessible/xpcom/xpcAccessibleHyperText.cpp:194:31: error: 'this' pointer is null [-Werror=nonnull]
[task 2021-05-19T00:00:35.299Z] 00:00:35     INFO -    194 |       props->SetStringProperty(attrs[i].Name(), attrs[i].Value(), unused);
[task 2021-05-19T00:00:35.299Z] 00:00:35     INFO -        |       ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2021-05-19T00:00:35.299Z] 00:00:35     INFO -  /builds/worker/checkouts/gecko/accessible/xpcom/xpcAccessibleHyperText.cpp: In member function 'virtual nsresult mozilla::a11y::xpcAccessibleHyperText::GetDefaultTextAttributes(nsIPersistentProperties**)':
[task 2021-05-19T00:00:35.299Z] 00:00:35    ERROR -  /builds/worker/checkouts/gecko/accessible/xpcom/xpcAccessibleHyperText.cpp:223:31: error: 'this' pointer is null [-Werror=nonnull]
[task 2021-05-19T00:00:35.299Z] 00:00:35     INFO -    223 |       props->SetStringProperty(attrs[i].Name(), attrs[i].Value(), unused);
[task 2021-05-19T00:00:35.300Z] 00:00:35     INFO -        |       ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2021-05-19T00:00:35.300Z] 00:00:35     INFO -  cc1plus: all warnings being treated as errors

The code in both cases, looks like:

nsCOMPtr<nsIPersistentProperties> props;
if (...) {
   props = ...
} else {
   ...
   props->SetStringProperty(...);
}

It blows my mind that this obvious null deref hasn't caused crashes on Mac and Linux in the 5 years that this code has been around.

Comment 1

[James Teh [:Jamie]]

(In reply to Mike Hommey [:glandium] from comment #0)

It blows my mind that this obvious null deref hasn't caused crashes on Mac and Linux in the 5 years that this code has been around.

The a11y XPCOM implementation is only used by automated tests. Clients talk to OS API implementations which call RemoteAccessible themselves. I guess we don't have any tests that exercise fetching text attributes across processes.

Comment 2

[Mike Hommey [:glandium]]

So I guess a way to fix this is to make all platforms use the windows variant of the code. https://searchfox.org/mozilla-central/rev/98a9257ca2847fad9a19631ac76199474516b31e/accessible/xpcom/xpcAccessibleHyperText.cpp#186

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1711811 - Remove code that dereferences an uninitialized pointer in xpcAccessibleHyperText::Get*TextAttributes.

Likely, the code is never used.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/685d5e96a39c
Remove code that dereferences an uninitialized pointer in xpcAccessibleHyperText::Get*TextAttributes. r=Jamie

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/685d5e96a39c