Alternative fix to #6468 #6475
Alternative fix to #6468 #6475
Conversation
nodejs-github-bot
added
the
v8 engine
indutny
force-pushed
the
feature/alt-gh-6466
branch
from
723a32c
to
102d478
Compare
|
cc @bnoordhuis |
indutny
added
macos
|
@indutny I like this approach. Can you propose this upstream? |
|
@ofrobots just did it: https://codereview.chromium.org/1934453003/ |
| msg.Append("shared-library,\"%s\",0x%08" V8PRIxPTR ",0x%08" V8PRIxPTR, | ||
| library_path.c_str(), start, end); | ||
| msg.Append("shared-library,\"%s\",0x%08" V8PRIxPTR ",0x%08" V8PRIxPTR | ||
| ",0x%08" V8PRIxPTR, |
There was a problem hiding this comment.
You should probably print the slide as a signed base 10 number because it can be < 0. For that matter, the code should really be updated to use intptr_t because that's what _dyld_get_image_vmaddr_slide() returns.
There was a problem hiding this comment.
Any pros of using unsigned int and base 10 here? I like how things may work both ways here.
There was a problem hiding this comment.
Also, may I ask you to raise this concern on CL page?
|
Left a comment. The approach in general looks fine to me. |
|
CL has just landed. Guess we should revisit this when it will gets to us with a v8 upgrade. |
|
Superseded by #6558 |
Checklist
Affected core subsystem(s)
deps
Description of change
Note: this PR should not be landed until this patch will be upstreamed to the v8's trunk.
Here I propose, instead of turning off ASLR at either runtime or compile-time, export the ASLR slide in the profile data and parse it to resolve the symbols during
--prof-process.Fix: #6466