From 64f86745b16987d630e1074f0f56099d552b5e52 Mon Sep 17 00:00:00 2001
From: Stephan Renatus <stephan.renatus@gmail.com>
Date: Mon, 21 Jun 2021 17:35:07 +0200
Subject: [PATCH] wasm: emit unreachable instruction after opa_abort() (#3574)

It's become apparent that a call to opa_abort in opa_agg_count did NOT
stop execution. Failures in bad input there haven't been thoroughly tested.

Comparing to the calls to opa_abort that happen in compiler-emitted code
(as opposed to calls in the C portion of our wasm code base), we find that
they are always emitting `unreachable` after the call to opa_abort.

As it turns out, doing the same thing in the C parts fixes the problem.
It thus seems like this is somehow related to changes in wasmtime's
cranelift (or its backend). It's never wrong to give the compiler some
more information that we can readily share, so let's put some `unreachable`
into the their proper places.

To avoid touching the entire code base of the C parts, we're defining a
function called `opa_abort(msg)` that'll call `opa_abort_(msg)` followed
by `__builtin_unreachable()`, which gives us that instruction. `opa_abort_`
in turn is imported as `opa_abort`, to keep compatibility with any SDKs
out there.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
---
 internal/wasm/sdk/opa/opa_test.go | 7 +++++++
 wasm/src/json.c                   | 1 +
 wasm/src/lib/stdio.c              | 1 +
 wasm/src/lib/stdlib.c             | 6 ++++++
 wasm/src/lib/stdlib.h             | 2 ++
 wasm/src/malloc.c                 | 1 +
 wasm/src/std.h                    | 1 -
 wasm/src/undefined.symbols        | 1 -
 8 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/internal/wasm/sdk/opa/opa_test.go b/internal/wasm/sdk/opa/opa_test.go
index 3cdbbc6a39..9664aa1d06 100644
--- a/internal/wasm/sdk/opa/opa_test.go
+++ b/internal/wasm/sdk/opa/opa_test.go
@@ -225,6 +225,13 @@ a = "c" { input > 2 }`,
 				{Result: `{{}}`},
 			},
 		},
+		{
+			Description: "Runtime error/bad utf8 input to count()",
+			Policy:      `a = count(base64.decode("2E84ZuPUd7zfvCZSNEchVpDEIj6PL7JfLpIqyxVG16k="))`,
+			Query:       "data.p.a = x",
+			Evals:       []Eval{{}},
+			WantErr:     "internal_error: string: invalid unicode",
+		},
 	}
 
 	for _, test := range tests {
diff --git a/wasm/src/json.c b/wasm/src/json.c
index cb0fd3d3a6..56f9e89adf 100644
--- a/wasm/src/json.c
+++ b/wasm/src/json.c
@@ -1,5 +1,6 @@
 #include <stdio.h>
 
+#include "stdlib.h"
 #include "str.h"
 #include "value.h"
 #include "json.h"
diff --git a/wasm/src/lib/stdio.c b/wasm/src/lib/stdio.c
index b7bfb0c9f3..2698d3febf 100644
--- a/wasm/src/lib/stdio.c
+++ b/wasm/src/lib/stdio.c
@@ -2,6 +2,7 @@
 #include "printf.h"
 
 #include "../std.h"
+#include "stdlib.h"
 
 struct _FILE {};
 
diff --git a/wasm/src/lib/stdlib.c b/wasm/src/lib/stdlib.c
index 75532f627a..801b57cb70 100644
--- a/wasm/src/lib/stdlib.c
+++ b/wasm/src/lib/stdlib.c
@@ -15,6 +15,12 @@ void abort(void)
     }
 }
 
+void opa_abort(const char *msg)
+{
+    opa_abort_(msg);
+    __builtin_unreachable();
+}
+
 void *malloc(size_t size)
 {
     return opa_malloc(size);
diff --git a/wasm/src/lib/stdlib.h b/wasm/src/lib/stdlib.h
index 12b87cc25d..b5987b50b2 100644
--- a/wasm/src/lib/stdlib.h
+++ b/wasm/src/lib/stdlib.h
@@ -7,6 +7,8 @@
 extern "C" {
 #endif
 
+void opa_abort(const char *msg);
+__attribute__((import_name("opa_abort"))) void opa_abort_(const char *msg);
 void abort(void);
 void *malloc(size_t size);
 void free(void *ptr);
diff --git a/wasm/src/malloc.c b/wasm/src/malloc.c
index b1544f138c..17b4594ef3 100644
--- a/wasm/src/malloc.c
+++ b/wasm/src/malloc.c
@@ -1,5 +1,6 @@
 #include <string.h>
 #include "std.h"
+#include "stdlib.h"
 
 #define WASM_PAGE_SIZE (65536)
 
diff --git a/wasm/src/std.h b/wasm/src/std.h
index 7df5513320..72b9656222 100644
--- a/wasm/src/std.h
+++ b/wasm/src/std.h
@@ -11,7 +11,6 @@ extern "C" {
 #define container_of(ptr, type, member)                             \
     ((type *)(void *)( ((char *)(ptr) - offsetof(type, member) )))
 
-void opa_abort(const char *msg);
 void opa_println(const char *msg);
 
 #ifdef DEBUG
diff --git a/wasm/src/undefined.symbols b/wasm/src/undefined.symbols
index d518633126..8ed7e08cef 100644
--- a/wasm/src/undefined.symbols
+++ b/wasm/src/undefined.symbols
@@ -1,4 +1,3 @@
-opa_abort
 opa_println
 opa_builtin0
 opa_builtin1