Closed
Bug 1458325
Opened 6 years ago
Closed 6 years ago
Patch llvm to fix Windows ASan jit-tests
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox62 fixed)
RESOLVED
FIXED
mozilla62
| Tracking | Status | |
|---|---|---|
| firefox62 | --- | fixed |
People
(Reporter: away, Assigned: away)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
|
1.34 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
My proposed patch is pending review at https://reviews.llvm.org/D46291
Attachment #8972422 -
Flags: review?(nfroyd)
|
||
Comment 2•6 years ago
|
||
Comment on attachment 8972422 [details] [diff] [review] highshadow Review of attachment 8972422 [details] [diff] [review]: ----------------------------------------------------------------- I'm not totally sure that the patch is obvious enough that we could commit this before LLVM reviews it. Then again, since the patch only (apparently?) matters on Win64, and we don't have any tests running on Win64, and getting it in probably makes things easier for you, your call on committing it prior to review.
Attachment #8972422 -
Flags: review?(nfroyd) → review+
I'm pretty confident that the change is ok, based on: - `PrintAddressSpaceLayout`: https://github.com/llvm-mirror/compiler-rt/blob/9ccd9976910686c34baf18493c0796458e826847/lib/asan/asan_rtl.cc#L321 - It looks like this was copy-pasted from a nearby function `AddrIsInHighMem` and not fixed up. - The jit-tests failed. We have a test that verifies that we trap on out-of-bounds wasm memory writes, but the bad write was being swallowed by ShadowExceptionHandler because ASan mistakenly thought the address was in shadow memory. Come to think of it, it looks like `AddrIsInMidShadow` has the same bug. It didn't surface in our jit-tests because our run doesn't use mid-shadow. I guess I'll fix it anyway.
Got an r+ on this so I'll push to svn and rename our patch from D- to r-.
Pushed by dmajor@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/afab7328e4db Merge ASan runtime patch to fix Win64 jit-tests. r=froydnj
Pushed by dmajor@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/60ab8ccf7a64 followup bustage fix
|
|
||
Comment 7•6 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/afab7328e4db https://hg.mozilla.org/mozilla-central/rev/60ab8ccf7a64
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
|
||
Updated•5 years ago
|
Version: Version 3 → 3 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•