Closed Bug 1711811 Opened 3 years ago Closed 3 years ago

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

Categories

(Firefox :: Disability Access, defect)

defect

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

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.

(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.

Severity: -- → S4

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

Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/685d5e96a39c
Remove code that dereferences an uninitialized pointer in xpcAccessibleHyperText::Get*TextAttributes. r=Jamie
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
Blocks: 1714004
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: