Review what output memory area should contain in case of failures in EEI methods

[axic]

Sparked by a discussion in https://github.com/ewasm/design/pull/76#discussion_r206594988

[lrettig]

When in doubt I'd prefer to err on the side of safety, given that this is Ethereum, and to me safety here means zeroing it out. This forces the developer to always be explicit.

[axic]

@chfast @holiman can you please raise your voices here?

[chfast]

I'm for leaving the memory untouched in case of failure.

Let's consider example methods:

bool get_something(int* output);
void do_something(int);

In case of option where the output is zeroed on failure, the following use cases are valid:

void use_case_1() {
    int x;
    if (get_something(&x))
        do_something(x);
}

void use_case_2() {
    int x;
    get_something(&x);
    do_something(x);
}

In case of option where the output is untouched, the use_case_2() is invalid and can be easily detected by static analysis tools. However the use_case_2() can be changed into a valid one by initializing int x = 0.

I would specify that the error code returned from methods SHOULD be checked. That would be "in text" equivalent of warn_unused_result function annotation in C/C++.

[holiman]

I would (tentatively) be for @lrettig 's suggestion.

When in doubt I'd prefer to err on the side of safety, given that this is Ethereum, and to me safety here means zeroing it out. This forces the developer to always be explicit.

Perhaps @chfast can expand a bit on the reasoning above, not sure I understand the implications of those examples.

[jakelang]

In the case of EEI methods that have a return value and take an output memory pointer, it definitely makes more sense to leave memory untouched.

However lots of EEI methods taking an output pointer as an argument don't have a return value, meaning that the caller can't trivially know if the call failed or not.

For comparison, malloc returns NULL on failure and sets errno. From C, error checking can then look somewhat like this: if ((c = malloc(size)) == NULL) { // handle error based on errno }

For us, lots of EEI methods (those with no return value but with an output data segment) can't signal failure without zeroing the output segment. But this reflects on a larger design flaw, not that we need to zero memory on failure.

[axic]

Can you point to an EEI method which can fail but has no means to signal that?

[jakelang]

@axic i guess i forgot getBlockHash has a return value. I still don't see any particular security benefits from zeroing out all the output memory on failure, especially if the EEI method can signal failure using the return code already.

[axic]

I agree with @chfast and @jakelang's point so will close this issue now, meaning everything stays as is.