Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add basic Intel SGX support #2219
base: master
Are you sure you want to change the base?
Add basic Intel SGX support #2219
Changes from all commits
836cccb
2521275
497dd46
e12db1e
bb8427a
f69ad97
1d6d59f
0ec10b7
167efd7
6bacf10
ab5f62d
ec5b638
994eb12
31af16f
c7ce26c
43fba82
307dba7
e09a947
60c27b9
9ca0bd7
fa687f6
c984c1f
abe247f
9ca1f0b
55bc9b6
42fdfef
1f43ea5
3fc9a1a
a6226b4
69d9561
0d7e9be
d5b4c8c
1e9402d
2688816
06181e7
430a2fb
345ab2d
ead8474
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should be removed from the squashed commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trivia: no double star in comment style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should just say "in duk_config.h", no other means are supported. So something like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be no need to undefine any of these. Only platform files are intended to define the Date provider defines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably won't work as expected: if this define appears here, it will prevent the emission of the default value for all platforms when duk_config.h default/forced options section is emitted. The problem is that the default/forced options section only knows that the define is forced in the preceding header, but it doesn't know (or care) that it is conditional to a platform.
So IMO this header should just say in its comments that
DUK_USE_MEMBASED_POINTER_ENCODING
is a required option for Intel SGX enclaves. There should be a better way to address this, but at present I don't think configure.py can do better.One possible solution would be for platforms to optionally have a config check snippet at the end of
duk_config.h
where they could check that required options are met, so that they could#error
out cleanly.