embench / embench-iot

The main Embench repository
https://www.embench.org/
GNU General Public License v3.0
248 stars 101 forks source link

nettle-sha256 does not work with CFI that enforces types #172

Closed irichter closed 1 year ago

irichter commented 1 year ago

When compiling nettle-sha256 with clang using the -fsanitize=cfi-icall flag, there is a type mismatch between the type of the first function argument in nettle_hash_init_func, nettle_hash_update_func, and nettle_hash_digest_func (void*) and the first argument in the functions that implement these prototypes -- sha256_init, sha256_update, and sha256_digest (struct sha256_ctx*).

As a result, when benchmark_body attempts to call nettle_sha256.init, which is typed as void (void*), but has a pointer to a function typed void (struct sha256_ctx*) instead, the runtime notices that type discrepancy prevents execution.

To allow this benchmark to run with this type of CFI, either the nettle_hash_XXX_func definitions must be changed to accept struct sha256_ctx* as their first arguments, or the implementing functions can be changed as below:

 void
-sha256_init (struct sha256_ctx*)
+sha256_init (void *ctx_v)
 {
+  struct sha256_ctx *ctx = ctx_v;
jeremybennett commented 1 year ago

Hi @irichter

Thanks for raising this issue.

One of the design principles behind Embench is that the constituent benchmarks should be "real" programs. We want to avoid a uniform set of benchmarks that are in some sense perfect.

We have to do some modification - for example to ensure that benchmarks work correctly on 16-bit platforms when compiled with standard C11. But we do not require perfection in standards compliance. I haven't run them with all the sanitizers that are available these days, but I would be fairly certain that this isn't the only problem of this type within the benchmark suite.

These sort of problems may reduce the scope for compiler optimization, but I don't believe they will cause incorrect execution. So this isn't a change we would make.

However...

You might like to propose this change to the designers of the Nettle suite, since it would improve the quality of their programs.

Hope this explanation makes sense, and thanks for sharing your thoughts. I'll leave the issue open for a short while in case you wish to comment further.

Jeremy Bennett

jeremybennett commented 1 year ago

Marking as closed since there are no further comments.