Closed
Bug 1411626
Opened 7 years ago
Closed 7 years ago
-Wclass-memaccess: clearing an object of non-trivial type 'struct mozilla::gl::GLContextSymbols'
Categories
(Core :: Graphics, defect, P3)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla58
People
(Reporter: Sylvestre, Assigned: jgilbert)
References
(Blocks 1 open bug)
Details
(Whiteboard: [gfx-noted])
Attachments
(1 file, 1 obsolete file)
In file included from /root/firefox-gcc-last/gfx/gl/GLContext.h:42:0, from /root/firefox-gcc-last/gfx/gl/GLContextGLX.h:10, from /root/firefox-gcc-last/gfx/gl/GLContextProviderGLX.cpp:33: /root/firefox-gcc-last/gfx/gl/GLContextSymbols.h: In member function 'void mozilla::gl::GLContextSymbols::Zero()': /root/firefox-gcc-last/gfx/gl/GLContextSymbols.h:35:49: error: 'void* memset(void*, int, size_t)' clearing an object of non-trivial type 'struct mozilla::gl::GLContextSymbols'; use assignment or value-initialization instead [-Werror=class-memaccess] memset(this, 0, sizeof(GLContextSymbols)); ^ /root/firefox-gcc-last/gfx/gl/GLContextSymbols.h:28:8: note: 'struct mozilla::gl::GLContextSymbols' declared here struct GLContextSymbols ^~~~~~~~~~~~~~~~ cc1plus: all warnings being treated as errors /root/firefox-gcc-last/config/rules.mk:1072: recipe for target 'GLContex
Updated•7 years ago
|
Assignee: nobody → bpostelnicu
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
I'm wondering if it would also be OK to initialize all of the members in ctor without memset and to leave memset only in the reset. I'm asking this since we really want to integrate this: 525063
Comment hidden (mozreview-request) |
Updated•7 years ago
|
status-firefox56:
--- → wontfix
status-firefox57:
--- → wontfix
Priority: -- → P3
Whiteboard: [gfx-noted]
Assignee | ||
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8922309 [details] Bug 1411626 - don't use memset on mozilla::gl::GLContextSymbols since it's not a trivial type. https://reviewboard.mozilla.org/r/193366/#review198820 Not like this.
Attachment #8922309 -
Flags: review?(jgilbert) → review-
Assignee | ||
Comment 6•7 years ago
|
||
This warning is spurious here, since we're within the lifetime of the object. (Both in the ctor or in a member function)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: bpostelnicu → jgilbert
Assignee | ||
Comment 8•7 years ago
|
||
I'm inferring that this is from GCC8-dev, which I don't trivially have available to test with. I'm going to discourage you from trying to fix warnings from pre-release compilers, but I'm fine with at least this change.
Flags: needinfo?(sledru)
Assignee | ||
Updated•7 years ago
|
Attachment #8922309 -
Attachment is obsolete: true
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8922593 [details] Bug 1411626 - Make GLContextSymbols a pure aggregate POD. - https://reviewboard.mozilla.org/r/193706/#review199508 ::: gfx/gl/GLContext.cpp:352 (Diff revision 1) > > ScopedGfxFeatureReporter reporter("GL Context"); > > if (!InitWithPrefixImpl(prefix, trygl)) { > // If initialization fails, zero the symbols to avoid hard-to-understand bugs. > - mSymbols.Zero(); > + mSymbols = {}; Aren't we doing here a byte copy of an rvalue object o mSymbols? Why don't we keet the Zero function and call it here? Since we've deleted the ctor this is a trivial class hence we will no longer have the issue with memset and gcc8. ::: gfx/gl/GLContext.cpp:2144 (Diff revision 1) > > if (!MakeCurrent()) { > NS_WARNING("MakeCurrent() failed during MarkDestroyed! Skipping GL object teardown."); > } > > - mSymbols.Zero(); > + mSymbols = {}; Same here as above.
Assignee | ||
Comment 10•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8922593 [details] Bug 1411626 - Make GLContextSymbols a pure aggregate POD. - https://reviewboard.mozilla.org/r/193706/#review199508 > Aren't we doing here a byte copy of an rvalue object o mSymbols? > Why don't we keet the Zero function and call it here? > Since we've deleted the ctor this is a trivial class hence we will no longer have the issue with memset and gcc8. The optimizer should pick up on this, and this isn't perf-sensitive anyway.
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8922593 [details] Bug 1411626 - Make GLContextSymbols a pure aggregate POD. - https://reviewboard.mozilla.org/r/193706/#review199720
Attachment #8922593 -
Flags: review?(bpostelnicu) → review+
Comment 12•7 years ago
|
||
Pushed by jgilbert@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9cdf605ea0f4 Make GLContextSymbols a pure aggregate POD. - r=andi
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9cdf605ea0f4
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•