Add basic Intel SGX support

[hf]

Duktape is great for embedded environments, and so I wanted to try and run it in Intel SGX. There is some special configuration needed and refactoring string formatting in a few places, but otherwise works just fine.

Summary of changes:

1. Add a new platform platform_intelsgx.
   The order is important to be immediately above Linux and Windows. Intel SGX enclaves are compiled under Linux or Windows with a special C library (tlibc; trusted libc) but otherwise are indistinguishable from other object files. Therefore the Linux and Windows detection will trigger.
2. Automatic detection that we're compiling an enclave is really tricky.
   There is no official documentation that describes if there is a macro you can check to see if compiling against tlibc; I did manage to find _TLIBC_CDECL_ and a few others from sys/cdefs.h which might be used for this purpose but I decided it's best to prefer manual versus automatic detection. In order to hint that this is the Intel SGX build either define INTELSGX manually or use --platform=intelsgx in the configure.py invocations.
3. Intel SGX enclaves do not have access to syscalls, and therefore they can't get information about the current time and locale timezone offset. This makes it impossible to run Duktape without any intervention, but luckily this boils down to defining two macros DUK_USE_DATE_GET_NOW and DUK_USE_DATE_GET_LOCAL_TZOFFSET. These are mandatory and well documented, and compilation fails with a preprocessor error if they are not added.
4. Intel SGX's tlibc does not include the "unsafe" string parsing and formatting functions. It only uses the "safe" ones. Meaning that all uses of DUK_SPRINTF needed to be refactored into DUK_SNPRINTF, but that was an easy affair. In general it's my opinion that going forward the unsafe functions be completely "banned" from the Duktape code, and be replaced with safe ones.
5. Add DUK_USE_STANDARDIZED_POINTER_ENCODING option (default: false) that encodes all pointers in a "standardized" way i.e. in lowercase hexadecimal encoding as the value is laid out in memory. In order to implement this, the functions duk_encode_pointer_cstr and duk_decode_pointer_cstr were added to duk_util_misc.c. They have also been replaced wherever the %p format specifier was used directly or indirectly (except for debug statements, of course). By enabling this option, Duktape no longer uses DUK_SSCANF.
6. Added a few things to .gitignore, most importantly runtests/package-lock.json which is added by npm when tests are run locally. It may be a good idea to remove this from .gitignore if we want to pin down versions.
7. Changed invocation of python within runtests/runtests.js to python2.7 since running the ecmatest with Python 3 failed with error. These days Python 3 is behind python, while 2.7 can be invoked with python2.7.

This PR is not complete I'm still testing and changing things, but discussions can begin IMO.

Remaining things to complete:

• apitest fails when DUK_USE_STANDARDIZED_POINTER_ENCODING because of the hardcoded expectation for 0xdeadbeef. ecmatest succeeds, tho. Possible solutions:
  1. Test expectations get passed through jade or some other preprocessor so they can be configured before being run.
  2. All 0xdeadbeef occurrences in expectations within tests/api are replaced with a configurable value.
  3. Change the standardized format, though I really like the simplicity of 1:1 memory layout in hex.
• Add new doc page for how to compile for Intel SGX.
• Write instructions for testing.

[hf]

Tests are failing because conditional undef DUK_USE_JX is not possible. The configuration tools read the undef but not the elif defined(DUK_F_INTELSGX) surrounding it.

[hf]

Here are some possible solutions to the problem:

1. Change reading and formatting pointers to functions for that purpose:

   #define DUK_PTR2STR(buf,sz,ptr) DUK_SNPRINTF(buf,sz,"%p",ptr)
   #define DUK_STR2PTR(buf,sz,ptr) DUK_SSCANF(buf,"%p",ptr)

   Then these can be set to different values for Intel SGX or other constrained environments.
2. Knowingly leave the DUK_SSCANF use in DUK_USE_JX sections, but add documentation and example configuration that disables DUK_USE_JX for Intel SGX.
3. Define platform independent encoding of pointers without the use of printf and scanf families.

[svaarala]

Thanks, interesting stuff! I'll look into the changes as soon as I get a chance.

I agree banning plain sprintf() would be nice. The reason sprintf() is currently used is that snprintf() has portability issues, e.g. it's not available on legacy platforms and sometimes interpretations before C99 differ, especially handling of truncation. So there are pros and cons in each case.

I'd be OK with requiring snprintf(), it's probably the better compromise. Users running pre-C99 would then need to implement or borrow one but that's not a big deal. This could be made easier by making it easy to use the minimal replacements https://github.com/svaarala/duktape/tree/master/extras/minimal-printf via config.

[hf]

I decided to add a new option DUK_USE_STANDARDIZED_POINTER_ENCODING which when enabled will encode pointers in a "standard" way, i.e. read the pointer value as it is laid out in memory and write it to a string in lowercase hex.

This PR is getting a little large, maybe once we agree on things I can break it up into smaller ones.

[hf]

Decided to add this safety check for sscanf because previously there was none, and there could have been cases where, for eg. { "__ptr": " " } was passed, and since sscanf and %p are generally available, but the %p is not specified meaning bad implementations may start reading the JX buffer until the end. This way, that can't ever happen.

[svaarala]

Re: forcing DUK_F_INTELSGX from outside, there are existing platforms where automatic detection is not possible and an external define is used to force detection, see for example https://github.com/svaarala/duktape/blob/master/config/helper-snippets/DUK_F_AMIGAOS.h.in (for VBCC).

It would be best for the define not to have a DUK_F_ prefix, because the configuration model uses those defines purely for internal purposes. Using just -DINTELSGX would be consistent with handling of e.g. Amiga + VBCC.

But if it would be clearer, maybe forced platforms could be handled using a convention like DUK_PLATFORM_AMIGA, DUK_PLATFORM_INTELSGX, etc.

[svaarala]

I'll try to review the diff tomorrow night, assuming it is ready for comments.

[hf]

Yes, ready for comments @svaarala.

I agree on the point for DUK_F_INTELSGX and will produce changes toward that approach this weekend.

[svaarala]

@hf I finally got some time to do a review on the changes. Mostly just trivia but there's one probable snprintf() truncation issue and AUTHORS needs a fix. With these fixed and the commits squashed this is ready for merging. Thanks for the detailed work! 👍

[hf]

@svaarala Thanks for reviewing. I tried to get all indent issues, but not sure how I missed these. Maybe think about setting up clang-format to get rid of issues like these?

I'll investigate more about the snprintf, and adjust code so that it works good. I like the MEMBASED name more, so I'll go with that one.

[hf]

What's the ideal style for void**? 😅
Is it void* *?

[hf]

Still many broken indents. Something is wrong with my vim, it's confused by this codebase.

[sva-p]

What's the ideal style for void**?

In the current codebase it would simply be void **.

[svaarala]

@svaarala Thanks for reviewing. I tried to get all indent issues, but not sure how I missed these. Maybe think about setting up clang-format to get rid of issues like these?

I've gone through uncrustify, astyle, and clang-format, and none of them have enough controls to get the results I would want (which is not surprising). But it's probably best to compromise on the result so that code style would be automatic, so I'll merge in something for 3.0.x release.

This was one major issue: https://stackoverflow.com/questions/38620019/can-clang-format-align-a-block-of-defines-for-me. It's fixed in clang-format-9 but that may not be available on the host so the autoindenting step will need to be dockerized.

[hf]

Also a good idea is to add .editorconfig since this codebase uses tabs (and I prefer spaces, but that is irrelevant). My vim struggled a bit, so I did a refactor of my dotfiles to fix it. :)

[svaarala]

Adding a .editorconfig sounds good to me. I'll steer clear of debating tabs vs spaces ;-)

[hf]

Hey @svaarala I finally managed to fix all outstanding issues. Would you mind checking the logic I used in the unresolved comments before merging?

TODOs post PR:

1. Add basic Intel SGX support #2219 (comment)

[svaarala]

I'll take a look - after that commits should be squashed for merging (a single commit is fine).

[svaarala]

This file should be removed from the squashed commit.

[svaarala]

Trivia: no double star in comment style.

[svaarala]

There should be no need to undefine any of these. Only platform files are intended to define the Date provider defines.

[svaarala]

This should just say "in duk_config.h", no other means are supported. So something like:

Enclave developers must provide DUK_USE_DATE_GET_NOW and DUK_USE_DATE_GET_LOCAL_TZOFFSET when configuring Duktape for build.

[svaarala]

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.

[svaarala]

Indent is broken here.

[svaarala]

This layout seems a bit confusing, the offsets in the bottom row are not continuous (due to len).

[svaarala]

This block has broken indent.

[svaarala]

Broken indent.

[svaarala]

I had forgotten about the possible negative error return value :o

Negative returns are probably not addressed correctly elsewhere. But it doesn't need to be addressed in this pull.

(Easiest fix would be to have an internal duk_snprintf() helper which would post-process the snprintf() return value: just convert <0 to 0, and cap the return value to bufsize - 1. The return value could then safely be used as is to increment pointers.)

[svaarala]

@hf Went through the changes and there's just some minor trivia left, see review comments.

Once you address them, you can just squash the commits into one, and I'll then merge the changes. I'll fix up whatever trivia is left while working on the snprintf() handling generally.