|
|
Created:
3 years, 6 months ago by fedor.indutny Modified:
3 years, 6 months ago CC:
v8-reviews_googlegroups.com Target Ref:
refs/heads/master Project:
v8 Visibility:
Public. |
Description[tickprocessor] fix ASLR slide use and `nm` on Mac
`libStart` already has ALSR slide added to it. Do not add it twice.
https: //codereview.chromium.org/2696903002/
Review-Url: https://codereview.chromium.org/2928083004
Cr-Commit-Position: refs/heads/master@{#46152}
Committed: https://chromium.googlesource.com/v8/v8/+/a8273f7e09f9315e46f8bdbf770b09b390bf7ba5
Patch Set 1 #Patch Set 2 : removed nm changes #Messages
Total messages: 18 (7 generated)
machenbach@chromium.org changed reviewers: + cbruni@chromium.org, jarin@chromium.org, machenbach@chromium.org
+jarin, since commit message claims this regressed since one of your CLs. Please tidy up BUG and R line in commit message.
As for the nm output - the symbol after the address is removed by the mac-nm script in the tools directory. How do you run the tick processor? Are you using the mac-tick-processor script? Or are you plugging in your custom nm? I am pretty sure that mac-tick-processor did not work before I hacked the scripts, and I am also sure it works now on my machine (and on Benedikt's mac, too).
Description was changed from ========== [tickprocessor] fix ASLR slide use and `nm` on Mac `libStart` already has ALSR slide added to it. Do not add it twice. `nm` still outputs the type of the symbol after its address. Do not let it leak into the symbol's name. This regression was caused by: BUG= R=machenbach https://codereview.chromium.org/2696903002/ ========== to ========== [tickprocessor] fix ASLR slide use and `nm` on Mac `libStart` already has ALSR slide added to it. Do not add it twice. `nm` still outputs the type of the symbol after its address. Do not let it leak into the symbol's name. This regression was caused by: https://codereview.chromium.org/2696903002/ ==========
On 2017/06/12 08:05:54, Jarin wrote: > As for the nm output - the symbol after the address is removed by the mac-nm > script in the tools directory. How do you run the tick processor? Are you using > the mac-tick-processor script? Or are you plugging in your custom nm? > We use `tickprocessor.js` directly. In fact, we bundle it inside of the node itself. I guess we can do extra pre-processing of `nm` output ourselves. > I am pretty sure that mac-tick-processor did not work before I hacked the > scripts, and I am also sure it works now on my machine (and on Benedikt's mac, > too).
Description was changed from ========== [tickprocessor] fix ASLR slide use and `nm` on Mac `libStart` already has ALSR slide added to it. Do not add it twice. `nm` still outputs the type of the symbol after its address. Do not let it leak into the symbol's name. This regression was caused by: https://codereview.chromium.org/2696903002/ ========== to ========== [tickprocessor] fix ASLR slide use and `nm` on Mac `libStart` already has ALSR slide added to it. Do not add it twice. https://codereview.chromium.org/2696903002/ ==========
Pushed changes and update commit message.
Kindly reminding about this. Thank you!
jarin, I'll leave this to you. I never touched this script.
machenbach@chromium.org changed reviewers: - machenbach@chromium.org, machenbach@google.com
Finally I remembered to test it out on my Mac. lgtm!
The CQ bit was checked by fedor@indutny.com
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498190283962010, "parent_rev": "4a48b6e52783fdb80737e966aef4da278a2e9dde", "commit_rev": "a8273f7e09f9315e46f8bdbf770b09b390bf7ba5"}
Message was sent while issue was closed.
Description was changed from ========== [tickprocessor] fix ASLR slide use and `nm` on Mac `libStart` already has ALSR slide added to it. Do not add it twice. https://codereview.chromium.org/2696903002/ ========== to ========== [tickprocessor] fix ASLR slide use and `nm` on Mac `libStart` already has ALSR slide added to it. Do not add it twice. https: //codereview.chromium.org/2696903002/ Review-Url: https://codereview.chromium.org/2928083004 Cr-Commit-Position: refs/heads/master@{#46152} Committed: https://chromium.googlesource.com/v8/v8/+/a8273f7e09f9315e46f8bdbf770b09b390b... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/v8/v8/+/a8273f7e09f9315e46f8bdbf770b09b390b...
Message was sent while issue was closed.
Thank you! |