Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(321)

Issue 10116004: Eliminate bundle_size parameter from validator interface. (Closed)

Created:
8 years, 8 months ago by Nick Bray (chromium)
Modified:
8 years, 8 months ago
CC:
native-client-reviews_googlegroups.com, pasko-google - do not use, Karl
Visibility:
Public.

Description

Eliminate bundle_size parameter from validator interface. Some superinstructions in src/trusted/validator_x86/ncval_tests.c were changed to work with 32 byte bundle sizes instead of 16 bytes. BUG= http://code.google.com/p/nativeclient/issues/detail?id=2737 TEST= none Committed: https://src.chromium.org/viewvc/native_client?view=rev&revision=8307

Patch Set 1 #

Total comments: 8

Patch Set 2 : rm 16-byte golden #

Patch Set 3 : Fix diff #

Patch Set 4 : Merge #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+90 lines, -725 lines) Patch
M src/tools/validator_tools/ncstubout.c View 3 chunks +2 lines, -3 lines 0 comments Download
M src/trusted/service_runtime/sel_validate_image.c View 4 chunks +2 lines, -6 lines 0 comments Download
M src/trusted/validator/ncfileutil.h View 1 chunk +0 lines, -1 line 0 comments Download
M src/trusted/validator/ncfileutil.c View 1 chunk +0 lines, -3 lines 0 comments Download
M src/trusted/validator/ncvalidate.h View 8 chunks +0 lines, -8 lines 0 comments Download
M src/trusted/validator/validation_cache_test.cc View 4 chunks +2 lines, -5 lines 0 comments Download
M src/trusted/validator/x86/32/ncvalidate.c View 2 4 chunks +3 lines, -13 lines 0 comments Download
M src/trusted/validator/x86/32/ncvalidate_verbose.c View 2 2 chunks +7 lines, -11 lines 0 comments Download
M src/trusted/validator/x86/64/ncvalidate.h View 2 1 chunk +0 lines, -1 line 0 comments Download
M src/trusted/validator/x86/64/ncvalidate.c View 1 2 3 6 chunks +3 lines, -12 lines 0 comments Download
M src/trusted/validator/x86/64/ncvalidate_verbose.c View 2 chunks +7 lines, -12 lines 0 comments Download
M src/trusted/validator/x86/ncval_reg_sfi/ncvalidate_iter.h View 2 chunks +0 lines, -2 lines 0 comments Download
M src/trusted/validator/x86/ncval_reg_sfi/ncvalidate_iter.c View 1 chunk +1 line, -3 lines 0 comments Download
M src/trusted/validator/x86/ncval_reg_sfi/ncvalidate_iter_detailed.h View 3 chunks +1 line, -3 lines 0 comments Download
M src/trusted/validator/x86/ncval_reg_sfi/ncvalidate_iter_detailed.c View 1 chunk +1 line, -3 lines 0 comments Download
M src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.h View 4 chunks +2 lines, -4 lines 0 comments Download
M src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c View 4 chunks +6 lines, -8 lines 1 comment Download
M src/trusted/validator/x86/ncval_seg_sfi/ncvalidate_detailed.h View 3 chunks +1 line, -3 lines 0 comments Download
M src/trusted/validator/x86/ncval_seg_sfi/ncvalidate_detailed.c View 1 chunk +1 line, -2 lines 0 comments Download
M src/trusted/validator_arm/ncvalidate.cc View 3 chunks +8 lines, -13 lines 0 comments Download
M src/trusted/validator_x86/build.scons View 3 chunks +2 lines, -9 lines 0 comments Download
M src/trusted/validator_x86/nccopycode.c View 3 chunks +14 lines, -24 lines 0 comments Download
M src/trusted/validator_x86/ncenuminsts_x86_32.c View 1 chunk +1 line, -1 line 0 comments Download
M src/trusted/validator_x86/ncenuminsts_x86_64.c View 2 chunks +3 lines, -3 lines 0 comments Download
M src/trusted/validator_x86/ncval.c View 8 chunks +8 lines, -17 lines 0 comments Download
M src/trusted/validator_x86/ncval_tests.c View 8 chunks +15 lines, -209 lines 4 comments Download
D src/trusted/validator_x86/testdata/32/inst-crosses.nvalds16 View 1 1 chunk +0 lines, -22 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/inst-crosses.nvals16 View 1 1 chunk +0 lines, -22 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-1.nvals16 View 1 1 chunk +0 lines, -26 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-11.nvals16 View 1 1 chunk +0 lines, -18 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-12.nvals16 View 1 1 chunk +0 lines, -18 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-14.nvals16 View 1 1 chunk +0 lines, -18 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-15.nvals16 View 1 1 chunk +0 lines, -19 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-18.nvalds16 View 1 1 chunk +0 lines, -19 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-18.nvals16 View 1 1 chunk +0 lines, -19 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-4.nvalds16 View 1 1 chunk +0 lines, -23 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-4.nvals16 View 1 1 chunk +0 lines, -23 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-5.nvalds16 View 1 1 chunk +0 lines, -23 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-5.nvals16 View 1 1 chunk +0 lines, -23 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-6.nvals16 View 1 1 chunk +0 lines, -18 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-7.nvals16 View 1 1 chunk +0 lines, -18 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-8.nvals16 View 1 1 chunk +0 lines, -18 lines 0 comments Download
D src/trusted/validator_x86/testdata/32/test-9.nvals16 View 1 1 chunk +0 lines, -19 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Nick Bray (chromium)
Apologies about the size of the CL, the roots went deeper than I anticipated. If ...
8 years, 8 months ago (2012-04-17 21:46:36 UTC) #1
Karl
https://chromiumcodereview.appspot.com/10116004/diff/1/src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c File src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c (right): https://chromiumcodereview.appspot.com/10116004/diff/1/src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c#newcode356 src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c:356: vstate->alignmask = alignment - 1; Is this safe? alignment ...
8 years, 8 months ago (2012-04-17 22:20:11 UTC) #2
Nick Bray (chromium)
https://chromiumcodereview.appspot.com/10116004/diff/1/src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c File src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c (right): https://chromiumcodereview.appspot.com/10116004/diff/1/src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c#newcode356 src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c:356: vstate->alignmask = alignment - 1; On 2012/04/17 22:20:12, Karl ...
8 years, 8 months ago (2012-04-17 22:42:02 UTC) #3
Karl
I'm willing to let Robert give the the thumbs up. https://chromiumcodereview.appspot.com/10116004/diff/1/src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c File src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c (right): https://chromiumcodereview.appspot.com/10116004/diff/1/src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c#newcode356 ...
8 years, 8 months ago (2012-04-17 22:48:01 UTC) #4
Nick Bray (chromium)
PTAL 90 insertions(+), 725 deletions(-) Craaaaazy.
8 years, 8 months ago (2012-04-17 23:15:19 UTC) #5
Robert Muth (chromium)
LGTM
8 years, 8 months ago (2012-04-18 20:22:35 UTC) #6
Mark Seaborn
8 years, 8 months ago (2012-04-18 22:58:20 UTC) #7
https://chromiumcodereview.appspot.com/10116004/diff/4070/src/trusted/validat...
File src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c (right):

https://chromiumcodereview.appspot.com/10116004/diff/4070/src/trusted/validat...
src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c:332: const int alignment =
32;
Should this be in a "#define NACL_BUNDLE_SIZE" somewhere?  Maybe shared between
the two x86 validators?

https://chromiumcodereview.appspot.com/10116004/diff/4070/src/trusted/validat...
File src/trusted/validator_x86/ncval_tests.c (right):

https://chromiumcodereview.appspot.com/10116004/diff/4070/src/trusted/validat...
src/trusted/validator_x86/ncval_tests.c:721: "83 e2 e0 \n"                      
/* and */
I'd say this sort of obscure change requires some explanation in the commit
message.

https://chromiumcodereview.appspot.com/10116004/diff/4070/src/trusted/validat...
src/trusted/validator_x86/ncval_tests.c:733: "83 e2 e0 \n"                      
/* and */
Ditto

https://chromiumcodereview.appspot.com/10116004/diff/4070/src/trusted/validat...
src/trusted/validator_x86/ncval_tests.c:996: exit(-1);
Can you use CHECK() instead?

https://chromiumcodereview.appspot.com/10116004/diff/4070/src/trusted/validat...
src/trusted/validator_x86/ncval_tests.c:1003: exit(-1);
Ditto

Powered by Google App Engine
This is Rietveld 408576698