structs that are uploaded should be `repr(C)` and implement `bytemuck::Pod`
Categories
(Core :: Graphics: WebRender, task, P3)
Tracking
()
People
(Reporter: nical, Assigned: glandium)
References
Details
Attachments
(1 obsolete file)
Structures that we pass to OpenGL directly via https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/device/gl.rs#3513 need to uphold a few guarantees like repr(C)
and that there is no padding in the data, otherwise it's UB.
Assignee | ||
Updated•5 months ago
|
Assignee | ||
Comment 1•5 months ago
|
||
Of all the things that end up on the function call you pointed out, only LineDecorationJob is missing a repr(C)
.
As for bytemuck::Pod
, it has two caveats:
- it requires Copy
- for everything that uses types from external crates, like Box2D, it needs to be implemented manually, which removes the advantage of the checks the derive macro does. Still better than nothing, though.
I guess the big question is whether all the affected types and all the types they contain indirectly are fine to be Copy.
List of the affected types: CopyInstance, BlurInstance, ScalingInstance, SvgFilterInstance, SVGFEFilterInstance, BorderInstance, ClipMaskInstanceRect, ClipMaskInstanceBoxShadow, PrimitiveInstanceData, CompositeInstance, ClearInstance, MaskInstance, ConicGradientInstance, FastLinearGradientInstance, LinearGradientInstance, RadialGradientInstance, LineDecorationJob, DebugFontVertex, DebugColorVertex, Box2D, Size2D, RenderTaskAddress, BlurDirection, GpuCacheAddress, Point2D, PremultipliedColorF, ClipMaskInstanceCommon, ClipData, BoxShadowData, TexelRect, CompositorTransform, TransformPaletteId, ClipSpace, Vector2D, ColorU. (but note that I haven't dug deeper than one level, so far)
Assignee | ||
Updated•5 months ago
|
Assignee | ||
Comment 2•5 months ago
•
|
||
mmmm, BlurDirection is an enum with 2 possible values. It's not Pod. Likewise for ClipSpace.
Assignee | ||
Comment 3•5 months ago
|
||
Oh, Box2D, and other types coming from euclid come with a bytemuck::Pod impl behind a feature.
Assignee | ||
Updated•5 months ago
|
Assignee | ||
Comment 4•5 months ago
|
||
This unveiled a couple UB-ish uses (missing repr(C) or use of enums).
Comment 6•5 months ago
|
||
bugherder |
Comment 7•5 months ago
|
||
Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/8a43e4098e24
Comment 9•5 months ago
|
||
Backed out as requested by glandium for causing test-linux1804-64-asan-qr/opt-mochitest-remote bustage.
- Backout link
- Push with failures
- Failure Log link
- Failure log:
[task 2024-05-23T06:09:57.927Z] 06:09:57 INFO - TEST-START | remote/shared/test/browser/browser_NavigationManager_no_navigation.js
[task 2024-05-23T06:09:59.719Z] 06:09:59 INFO - GECKO(11290) | 1716444599717 RemoteAgent TRACE [34] NavigationListener onLocationChange, location: https://example.com/document-builder.sjs?html=test
[task 2024-05-23T06:09:59.991Z] 06:09:59 INFO - GECKO(11290) | 1716444599990 RemoteAgent TRACE [34] NavigationListener onStateChange, stateFlags: 983041, status: 0, isStart: true, isStop: false, isNetwork: true, isBindingAborted: false, targetURI: https://example.com/document-builder.sjs?html=test
[task 2024-05-23T06:10:00.002Z] 06:10:00 INFO - GECKO(11290) | 1716444600001 RemoteAgent TRACE [9822135f-2ed0-4362-980b-6b973048fc7e] Navigation started for url: https://example.com/document-builder.sjs?html=test (90045d8e-1b99-452a-a612-d0b66037e897)
[task 2024-05-23T06:10:00.239Z] 06:10:00 INFO - GECKO(11290) | 1716444600238 RemoteAgent TRACE [34] NavigationListener onStateChange, stateFlags: 196610, status: 0, isStart: false, isStop: false, isNetwork: false, isBindingAborted: false, targetURI: https://example.com/document-builder.sjs?html=test
[task 2024-05-23T06:10:00.332Z] 06:10:00 INFO - GECKO(11290) | 1716444600330 RemoteAgent TRACE [34] NavigationListener onStateChange, stateFlags: 196612, status: 0, isStart: false, isStop: false, isNetwork: false, isBindingAborted: false, targetURI: https://example.com/document-builder.sjs?html=test
[task 2024-05-23T06:10:00.341Z] 06:10:00 INFO - GECKO(11290) | 1716444600331 RemoteAgent TRACE [34] NavigationListener onStateChange, stateFlags: 196612, status: 0, isStart: false, isStop: false, isNetwork: false, isBindingAborted: false, targetURI: https://example.com/document-builder.sjs?html=test
[task 2024-05-23T06:10:00.453Z] 06:10:00 INFO - GECKO(11290) | 1716444600452 RemoteAgent TRACE [34] NavigationListener onStateChange, stateFlags: 131088, status: 0, isStart: false, isStop: true, isNetwork: false, isBindingAborted: false, targetURI: https://example.com/document-builder.sjs?html=test
[task 2024-05-23T06:10:00.477Z] 06:10:00 INFO - GECKO(11290) | 1716444600476 RemoteAgent TRACE [9822135f-2ed0-4362-980b-6b973048fc7e] Navigation finished for url: https://example.com/document-builder.sjs?html=test (90045d8e-1b99-452a-a612-d0b66037e897)
[task 2024-05-23T06:10:51.413Z] 06:10:51 INFO - GECKO(11290) | LLVM ERROR: out of memory
[task 2024-05-23T06:10:51.413Z] 06:10:51 INFO - GECKO(11290) | Allocation failed
[task 2024-05-23T06:10:51.413Z] 06:10:51 ERROR - GECKO(11290) | ==11368==ERROR: AddressSanitizer: out of memory: failed to allocate 0xe000 (57344) bytes of Create (error code: 12)
[task 2024-05-23T06:10:51.414Z] 06:10:51 INFO - GECKO(11290) | ERROR: Failed to mmap
[task 2024-05-23T06:10:51.414Z] 06:10:51 INFO - GECKO(11290) | XPCOMGlueLoad error for file /builds/worker/workspace/build/application/firefox/libmozgtk.so:
[task 2024-05-23T06:10:51.414Z] 06:10:51 INFO - GECKO(11290) | libgcrypt.so.20: failed to map segment from shared object
[task 2024-05-23T06:10:51.581Z] 06:10:51 INFO - GECKO(11290) | PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
[task 2024-05-23T06:10:51.581Z] 06:10:51 INFO - GECKO(11290) | Stack dump:
[task 2024-05-23T06:10:51.718Z] 06:10:51 INFO - GECKO(11290) | 0. Program arguments: /builds/worker/workspace/build/application/firefox/llvm-symbolizer --demangle --inlines --default-arch=x86_64
[task 2024-05-23T06:17:46.662Z] 06:17:46 INFO - GECKO(11290) | Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
[task 2024-05-23T06:17:47.189Z] 06:17:47 INFO - Buffered messages logged at 06:09:57
[task 2024-05-23T06:17:47.189Z] 06:17:47 INFO - Entering test bound testDocumentOpenWriteClose
[task 2024-05-23T06:17:47.190Z] 06:17:47 INFO - Buffered messages logged at 06:09:58
[task 2024-05-23T06:17:47.192Z] 06:17:47 INFO - Console message: [JavaScript Warning: "This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use “<!DOCTYPE html>”." {file: "https://example.com/document-builder.sjs?html=test" line: 0}]
[task 2024-05-23T06:17:47.192Z] 06:17:47 INFO - Buffered messages logged at 06:09:59
[task 2024-05-23T06:17:47.192Z] 06:17:47 INFO - TEST-PASS | remote/shared/test/browser/browser_NavigationManager_no_navigation.js | No event recorded -
[task 2024-05-23T06:17:47.192Z] 06:17:47 INFO - Replace the document
[task 2024-05-23T06:17:47.193Z] 06:17:47 INFO - Console message: [JavaScript Warning: "This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use “<!DOCTYPE html>”." {file: "eval" line: 3}]
[task 2024-05-23T06:17:47.193Z] 06:17:47 INFO - TEST-PASS | remote/shared/test/browser/browser_NavigationManager_no_navigation.js | No event recorded after replacing the document -
[task 2024-05-23T06:17:47.193Z] 06:17:47 INFO - Reload the page, which should trigger a navigation
[task 2024-05-23T06:17:47.193Z] 06:17:47 INFO - Buffered messages logged at 06:10:00
[task 2024-05-23T06:17:47.193Z] 06:17:47 INFO - Console message: [JavaScript Warning: "This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use “<!DOCTYPE html>”." {file: "https://example.com/document-builder.sjs?html=test" line: 0}]
[task 2024-05-23T06:17:47.193Z] 06:17:47 INFO - TEST-PASS | remote/shared/test/browser/browser_NavigationManager_no_navigation.js | Recorded navigation events -
[task 2024-05-23T06:17:47.193Z] 06:17:47 INFO - Leaving test bound testDocumentOpenWriteClose
[task 2024-05-23T06:17:47.193Z] 06:17:47 INFO - Buffered messages finished
[task 2024-05-23T06:17:47.193Z] 06:17:47 INFO - TEST-UNEXPECTED-TIMEOUT | remote/shared/test/browser/browser_NavigationManager_no_navigation.js | application timed out after 370 seconds with no output
[task 2024-05-23T06:17:47.193Z] 06:17:47 INFO - TEST-INFO took 468079ms
[task 2024-05-23T06:17:47.478Z] 06:17:47 INFO - Buffered messages finished
[task 2024-05-23T06:17:47.478Z] 06:17:47 WARNING - Force-terminating active process(es).
[task 2024-05-23T06:17:47.478Z] 06:17:47 INFO - Determining child pids from psutil...
[task 2024-05-23T06:17:47.627Z] 06:17:47 INFO - [11368, 11396, 11441, 11449, 11451, 11469, 11492, 11555, 11571, 11603, 11604, 11607, 11663, 11690, 11712, 11755, 11757, 11804, 11805, 11860]
[task 2024-05-23T06:17:48.890Z] 06:17:48 INFO - ==> process 11290 launched child process 11368
[task 2024-05-23T06:17:48.890Z] 06:17:48 INFO - ==> process 11290 launched child process 11396
[task 2024-05-23T06:17:48.891Z] 06:17:48 INFO - ==> process 11290 launched child process 11441
<...>
[task 2024-05-23T06:19:38.940Z] 06:19:38 INFO - GECKO(12348) | console.error: ({})
[task 2024-05-23T06:19:42.542Z] 06:19:42 INFO - GECKO(12348) | [ERROR error_support::handling] suggest-unexpected: Error from Remote Settings: Error parsing URL: relative URL with a cannot-be-a-base base
[task 2024-05-23T06:19:42.559Z] 06:19:42 INFO - GECKO(12348) | console.error: URLBar - QuickSuggest.SuggestBackendRust: "Ingest error: Error from Remote Settings: Error parsing URL: relative URL with a cannot-be-a-base base"
[task 2024-05-23T06:19:42.906Z] 06:19:42 INFO - GECKO(12348) | 1716445182905 Marionette TRACE Received observer notification browser-idle-startup-tasks-finished
[task 2024-05-23T06:19:42.941Z] 06:19:42 INFO - GECKO(12348) | 1716445182940 RemoteAgent TRACE [9] ProgressListener Start: expectNavigation=false resolveWhenStarted=false unloadTimeout=40000 waitForExplicitStart=false
[task 2024-05-23T06:19:42.944Z] 06:19:42 INFO - GECKO(12348) | 1716445182941 RemoteAgent TRACE [9] ProgressListener Setting unload timer (40000ms)
[task 2024-05-23T06:19:42.944Z] 06:19:42 INFO - GECKO(12348) | 1716445182941 RemoteAgent TRACE [9] Wait for initial navigation: isInitial=false, isLoadingDocument=false
[task 2024-05-23T06:19:42.945Z] 06:19:42 INFO - GECKO(12348) | 1716445182942 RemoteAgent TRACE [9] Document already finished loading: about:blank
[task 2024-05-23T06:19:42.945Z] 06:19:42 INFO - GECKO(12348) | 1716445182943 RemoteAgent TRACE [9] ProgressListener Stop: has error=false url=about:blank
[task 2024-05-23T06:19:42.993Z] 06:19:42 INFO - GECKO(12348) | 1716445182991 Marionette DEBUG 3 <- [1,1,null,{"sessionId":"ad54992c-35d4-4e75-96ef-c7332788d652","capabilities":{"browserName":"firefox","browserVersion":"128.0a1","platformName":"linux","acceptInsecureCerts":false,"pageLoadStrategy":"normal","setWindowRect":true,"timeouts":{"implicit":0,"pageLoad":300000,"script":30000},"strictFileInteractability":true,"unhandledPromptBehavior":"dismiss and notify","userAgent":"Mozilla/5.0 (X11; Linux x86_64; rv:128.0) Gecko/20100101 Firefox/128.0","moz:accessibilityChecks":false,"moz:buildID":"20240523043730","moz:headless":false,"moz:platformVersion":"4.4.0-1014-aws","moz:processID":12348,"moz:profile":"/tmp/tmp9kjme047.mozrunner","moz:shutdownTimeout":300000,"moz:webdriverClick":true,"moz:windowless":false,"proxy":{}}}]
[task 2024-05-23T06:19:43.078Z] 06:19:43 INFO - GECKO(12348) | 1716445183077 Marionette DEBUG 3 -> [0,2,"Addon:Install",{"path":"/tmp/tmpj67glkob.zip","temporary":false}]
[task 2024-05-23T06:19:43.465Z] 06:19:43 INFO - GECKO(12348) | 1716445183464 Marionette DEBUG 3 <- [1,2,null,{"value":"special-powers@mozilla.org"}]
[task 2024-05-23T06:19:43.558Z] 06:19:43 INFO - GECKO(12348) | 1716445183555 Marionette DEBUG 3 -> [0,3,"Addon:Install",{"path":"/tmp/tmp5f1zc0fb.zip","temporary":false}]
[task 2024-05-23T06:19:43.665Z] 06:19:43 INFO - GECKO(12348) | 1716445183664 Marionette DEBUG 3 <- [1,3,null,{"value":"mochikit@mozilla.org"}]
[task 2024-05-23T06:19:43.699Z] 06:19:43 INFO - GECKO(12348) | 1716445183698 Marionette DEBUG 3 -> [0,4,"Marionette:GetContext",{}]
[task 2024-05-23T06:19:43.700Z] 06:19:43 INFO - GECKO(12348) | 1716445183698 Marionette DEBUG 3 <- [1,4,null,{"value":"content"}]
[task 2024-05-23T06:19:43.702Z] 06:19:43 INFO - GECKO(12348) | 1716445183701 Marionette DEBUG 3 -> [0,5,"Marionette:SetContext",{"value":"chrome"}]
[task 2024-05-23T06:19:43.703Z] 06:19:43 INFO - GECKO(12348) | 1716445183701 Marionette DEBUG 3 <- [1,5,null,{"value":null}]
[task 2024-05-23T06:19:43.728Z] 06:19:43 INFO - GECKO(12348) | 1716445183727 Marionette DEBUG 3 -> [0,6,"WebDriver:ExecuteScript",{"script":"/* This Source Code Form is subject to the terms of the Mozilla Public\n * License, v. 2.0. If a copy of the MPL was not distr ... s which flavor and url to load.\nlet ev = new CustomEvent(\"mochitest-load\", { detail: [flavor, url] });\nwin.dispatchEvent(ev);","args":[{"flavor":"browser-chrome","testUrl":"about:blank"}],"newSandbox":true,"sandbox":"default","line":2167,"filename":"tests/mochitest/runtests.py"}]
[task 2024-05-23T06:19:43.740Z] 06:19:43 INFO - GECKO(12348) | 1716445183739 RemoteAgent TRACE WebDriverProcessData actor created for PID 12348
[task 2024-05-23T06:19:43.743Z] 06:19:43 INFO - GECKO(12348) | 1716445183742 Marionette TRACE [1] MarionetteCommands actor created for window id 2
[task 2024-05-23T06:19:43.791Z] 06:19:43 INFO - GECKO(12348) | 1716445183790 RemoteAgent TRACE Received observer notification domwindowopened
[task 2024-05-23T06:19:43.806Z] 06:19:43 INFO - GECKO(12348) | 1716445183805 Marionette DEBUG 3 <- [1,6,null,{"value":null}]
[task 2024-05-23T06:19:43.847Z] 06:19:43 INFO - GECKO(12348) | 1716445183846 Marionette DEBUG 3 -> [0,7,"Marionette:SetContext",{"value":"content"}]
[task 2024-05-23T06:19:43.850Z] 06:19:43 INFO - GECKO(12348) | 1716445183847 Marionette DEBUG 3 <- [1,7,null,{"value":null}]
[task 2024-05-23T06:19:43.938Z] 06:19:43 INFO - GECKO(12348) | 1716445183937 Marionette DEBUG 3 -> [0,8,"WebDriver:DeleteSession",{}]
[task 2024-05-23T06:19:43.944Z] 06:19:43 INFO - GECKO(12348) | 1716445183939 Marionette TRACE [1] MarionetteCommands actor destroyed for window id 2
[task 2024-05-23T06:19:43.982Z] 06:19:43 INFO - GECKO(12348) | 1716445183981 Marionette DEBUG 3 <- [1,8,null,{"value":null}]
[task 2024-05-23T06:19:43.985Z] 06:19:43 INFO - runtests.py | Waiting for browser...
[task 2024-05-23T06:19:43.993Z] 06:19:43 INFO - GECKO(12348) | 1716445183985 Marionette DEBUG Closed connection 3
[task 2024-05-23T06:19:44.969Z] 06:19:44 INFO - *** Start BrowserChrome Test Results ***
[task 2024-05-23T06:19:45.081Z] 06:19:45 INFO - checking window state
[task 2024-05-23T06:19:45.235Z] 06:19:45 INFO - TEST-START | remote/webdriver-bidi/test/browser/browser_RemoteValue.js
Assignee | ||
Comment 10•5 months ago
|
||
This is quite baffling... enabling the bytemuck feature of euclid is what triggers whatever this is.
Assignee | ||
Comment 11•5 months ago
|
||
What do you think of making it optional and only built on e.g. wrench builds? That would catch UB, while avoiding whatever it is that is happening, as well as avoiding the extra derive(Copy) on many types.
Reporter | ||
Comment 12•5 months ago
|
||
Wow!
On second thought, I suspect that not using Pod, while a bit brittle, is actually fine given the way we currently use these types. However the missing #[repr(C)]
is important and addresses real UB, so we can at least extract that part and land it.
About optionally requiring Pod, I suspect that it's going to cause more friction than its worth but I don't feel very strongly, I'll ask around.
Assignee | ||
Comment 13•5 months ago
|
||
so we can at least extract that part and land it.
Already did in bug 1898614
Comment 14•4 months ago
|
||
There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:glandium, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.
Assignee | ||
Comment 15•4 months ago
|
||
(In reply to Nicolas Silva [:nical] from comment #12)
About optionally requiring Pod, I suspect that it's going to cause more friction than its worth but I don't feel very strongly, I'll ask around.
Should this be WONTFIX at this point?
Reporter | ||
Comment 16•4 months ago
|
||
Yeah, let's move on. Maybe we'll try again later and whatever caused the mysterious asan issue won't happen then. In the mean time it's not important enough for us to spend to time figuring it out.
Updated•4 months ago
|
Description
•