Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2179303003:
[gn] Don't use PIE for host executables
|
Created:
4 years, 4 months ago by Michael Achenbach Modified:
4 years, 4 months ago CC:
oth, ghost stip (do not use), v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[gn] Don't use PIE for host executables
Using PIE switches on ASLR. With mksnapshot, this can lead
to non-deterministic snapshots.
BUG=v8:5233
Committed: https://crrev.com/4ca39b53245619d94a80a93939613774e68e4649
Cr-Commit-Position: refs/heads/master@{#38084}
Patch Set 1 #Patch Set 2 : [gn] Don't use PIE for host executables #
Total comments: 3
Patch Set 3 : Fix #
Total comments: 1
Messages
Total messages: 28 (14 generated)
Description was changed from ========== [gn] Don't use PIE for host executables BUG= ========== to ========== [gn] Don't use PIE for host executables BUG=v8:5233 ==========
machenbach@chromium.org changed reviewers: + jochen@chromium.org, vogelheim@chromium.org, yangguo@chromium.org
PTAL
Checked output of: gn desc out/exp ":mksnapshot(//build/toolchain/linux:clang_x86_v8_arm)"
Description was changed from ========== [gn] Don't use PIE for host executables BUG=v8:5233 ========== to ========== [gn] Don't use PIE for host executables Using PIE switches on ASLR. With mksnapshot, this can lead to non-deterministic snapshots. BUG=v8:5233 ==========
Note that https://codereview.chromium.org/2178093003/ is fixing the root cause of non-determinism here. But switching off ASLR for host executables might still be a good idea to prevent non-determinism in the future.
On 2016/07/26 08:40:15, Michael Achenbach (slow) wrote: > Note that https://codereview.chromium.org/2178093003/ is fixing the root cause > of non-determinism here. But switching off ASLR for host executables might still > be a good idea to prevent non-determinism in the future. lgtm.
https://codereview.chromium.org/2179303003/diff/20001/gni/v8.gni File gni/v8.gni (right): https://codereview.chromium.org/2179303003/diff/20001/gni/v8.gni#newcode94 gni/v8.gni:94: if (!defined(invoker.want_pie) || invoker.want_pie) { instead of using want_pie, why not just never using -pie when targetting the host?
https://codereview.chromium.org/2179303003/diff/20001/gni/v8.gni File gni/v8.gni (right): https://codereview.chromium.org/2179303003/diff/20001/gni/v8.gni#newcode94 gni/v8.gni:94: if (!defined(invoker.want_pie) || invoker.want_pie) { On 2016/07/27 07:41:31, jochen wrote: > instead of using want_pie, why not just never using -pie when targetting the > host? I don't know here if I target the host. I only know which toolchain is used. If that's the same for host and target (e.g. a normal x64 build) I still don't want pie for mksnapshot, but I want it for d8. How would I distinguish the two?
fair enough, lgtm
The CQ bit was checked by machenbach@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [gn] Don't use PIE for host executables Using PIE switches on ASLR. With mksnapshot, this can lead to non-deterministic snapshots. BUG=v8:5233 ========== to ========== [gn] Don't use PIE for host executables Using PIE switches on ASLR. With mksnapshot, this can lead to non-deterministic snapshots. BUG=v8:5233 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [gn] Don't use PIE for host executables Using PIE switches on ASLR. With mksnapshot, this can lead to non-deterministic snapshots. BUG=v8:5233 ========== to ========== [gn] Don't use PIE for host executables Using PIE switches on ASLR. With mksnapshot, this can lead to non-deterministic snapshots. BUG=v8:5233 Committed: https://crrev.com/4ca39b53245619d94a80a93939613774e68e4649 Cr-Commit-Position: refs/heads/master@{#38084} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/4ca39b53245619d94a80a93939613774e68e4649 Cr-Commit-Position: refs/heads/master@{#38084}
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2187613003/ by hablich@chromium.org. The reason for reverting is: Breaks roll: https://codereview.chromium.org/2182043004/.
Message was sent while issue was closed.
https://codereview.chromium.org/2179303003/diff/20001/gni/v8.gni File gni/v8.gni (right): https://codereview.chromium.org/2179303003/diff/20001/gni/v8.gni#newcode101 gni/v8.gni:101: configs -= "//build/config/android:executable_config" Meh: sorry for the half-baked CL. Forgot []
Message was sent while issue was closed.
Description was changed from ========== [gn] Don't use PIE for host executables Using PIE switches on ASLR. With mksnapshot, this can lead to non-deterministic snapshots. BUG=v8:5233 Committed: https://crrev.com/4ca39b53245619d94a80a93939613774e68e4649 Cr-Commit-Position: refs/heads/master@{#38084} ========== to ========== [gn] Don't use PIE for host executables Using PIE switches on ASLR. With mksnapshot, this can lead to non-deterministic snapshots. BUG=v8:5233 Committed: https://crrev.com/4ca39b53245619d94a80a93939613774e68e4649 Cr-Commit-Position: refs/heads/master@{#38084} ==========
PTAL at patch 3. Fix + (unfortunately also) rebase. https://codereview.chromium.org/2179303003/diff/40001/gni/v8.gni File gni/v8.gni (right): https://codereview.chromium.org/2179303003/diff/40001/gni/v8.gni#newcode101 gni/v8.gni:101: configs -= [ "//build/config/android:executable_config" ] This is the only change in patch 3
The CQ bit was checked by machenbach@chromium.org to run a CQ dry run
The CQ bit was unchecked by machenbach@chromium.org
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by machenbach@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Hold on. Looks like I need to fix something according to the trybots... |
