runtime, tests: require -Werror=strict-prototypes so every function has a prototype

kkysen · kkysen · commit 657aafb25a6f · 2024-12-18T10:49:55.000-08:00
In order to have C callgate wrappers, we need to know function prototypes,
but `ReturnType foo()` functions have no prototype, as `foo(void)` is required.

ABI determination (and soon to be API, too) is done eagerly for all functions,
so this errors when a function has no prototype.
We can later relax this to only requiring prototypes on functions with callgates.

Furthermore, often projects enable `-Werror=strict-prototypes` themselves, like `dav1d`.
Turning this on here for the IA2 runtime and tests means that
we don't have to disable it in other projects like `dav1d`.
diff --git a/cmake/ia2.cmake b/cmake/ia2.cmake
@@ -52,6 +52,7 @@ function(add_ia2_compartment NAME TYPE)
   set_target_properties(${NAME} PROPERTIES PKEY ${ARG_PKEY})
   target_compile_options(${NAME} PRIVATE
     "-Werror=incompatible-pointer-types"
+    "-Werror=strict-prototypes"
     "-fPIC"
   )
 
diff --git a/docs/compartmentalizing_dav1d.md b/docs/compartmentalizing_dav1d.md
@@ -151,7 +151,6 @@ Then there are overrides:
 First just disabling some warnings/errors that `dav1d` enabled but IA2's runtime doesn't follow
 (so this would be different for another project):
 
-* `-Wno-strict-prototypes`
 * `-Wno-missing-prototypes`
 * `-Wno-unused-function`
 * `-Wno-unknown-warning-option`
diff --git a/docs/usage.md b/docs/usage.md
@@ -205,6 +205,7 @@ are also required:
 -I $IA2_PATH/runtime/libia2/include
 -I $IA2_PATH/runtime/partition-alloc/include
 -Werror=incompatible-pointer-types
+-Werror=strict-prototypes
 -Wl,--wrap=pthread_create
 -Wl,--wrap=calloc
 -Wl,--wrap=free
diff --git a/runtime/libia2/include/ia2.h b/runtime/libia2/include/ia2.h
@@ -152,10 +152,10 @@ extern "C" {
 #endif
 
 /// Returns the raw PKRU register value
-uint32_t ia2_get_pkru();
+uint32_t ia2_get_pkru(void);
 
 /// Returns the current compartment pkey
-size_t ia2_get_pkey();
+size_t ia2_get_pkey(void);
 
 #ifdef __cplusplus
 }
diff --git a/runtime/libia2/include/ia2_compartment_init.inc b/runtime/libia2/include/ia2_compartment_init.inc
@@ -45,7 +45,7 @@ extern char __start_ia2_shared_data __attribute__((visibility("hidden"))),
     __stop_ia2_shared_data __attribute__((visibility("hidden")));
 
 void ia2_set_up_tags(int *n_to_alloc);
-__attribute__((constructor)) static void COMPARTMENT_IDENT(init_pkey)() {
+__attribute__((constructor)) static void COMPARTMENT_IDENT(init_pkey)(void) {
   ia2_set_up_tags(&ia2_n_pkeys_to_alloc);
   struct IA2SharedSection shared_sections[2] = {{
                                                     &__start_ia2_shared_data,
diff --git a/runtime/libia2/include/ia2_internal.h b/runtime/libia2/include/ia2_internal.h
@@ -339,7 +339,7 @@ static int ia2_mprotect_with_tag(void *addr, size_t len, int prot, int tag) {
 #endif
 #endif
 char *allocate_stack(int i);
-void allocate_stack_0();
+void allocate_stack_0(void);
 void verify_tls_padding(void);
 void ia2_set_up_tags(int *n_to_alloc);
 __attribute__((__noreturn__)) void ia2_reinit_stack_err(int i);
diff --git a/runtime/libia2/init.c b/runtime/libia2/init.c
@@ -36,7 +36,7 @@ char *allocate_stack(int i) {
   return stack + STACK_SIZE - 8;
 }
 
-void allocate_stack_0() {
+void allocate_stack_0(void) {
   ia2_stackptr_0[0] = allocate_stack(0);
 }
 
diff --git a/runtime/partition-alloc/src/get_pkey.cc b/runtime/partition-alloc/src/get_pkey.cc
@@ -6,14 +6,14 @@
 
 #ifdef __x86_64__
 __attribute__((__visibility__("hidden")))
-uint32_t ia2_get_pkru() {
+uint32_t ia2_get_pkru(void) {
   uint32_t pkru = 0;
   __asm__ volatile("rdpkru" : "=a"(pkru) : "a"(0), "d"(0), "c"(0));
   return pkru;
 }
 
 __attribute__((__visibility__("hidden")))
-size_t ia2_get_pkey() {
+size_t ia2_get_pkey(void) {
   uint32_t pkru;
   __asm__("rdpkru" : "=a"(pkru) : "a"(0), "d"(0), "c"(0));
   switch (pkru) {
diff --git a/tests/abi/abi.c b/tests/abi/abi.c
@@ -12,12 +12,12 @@ void arg1(int x) {
 }
 
 // LINKARGS: --wrap=foo
-void foo() {
+void foo(void) {
     cr_log_info("foo");
 }
 
 // LINKARGS: --wrap=return_val
-int return_val() {
+int return_val(void) {
     cr_log_info("return_val");
     return 1;
 }
diff --git a/tests/abi/include/abi.h b/tests/abi/include/abi.h
@@ -5,10 +5,10 @@ struct in_memory {
 };
 
 // This function does nothing
-void foo();
+void foo(void);
 
 // This returns an integer 1
-int return_val();
+int return_val(void);
 
 // This takes an integer, expects value 1
 void arg1(int x);
diff --git a/tests/header_includes/include/liboption.h b/tests/header_includes/include/liboption.h
@@ -4,4 +4,4 @@
 
 Option Some(int x);
 
-Option None();
+Option None(void);
diff --git a/tests/header_includes/liboption.c b/tests/header_includes/liboption.c
@@ -7,7 +7,7 @@ RUN: cat header_includes_call_gates_1.ld | FileCheck --check-prefix=LINKARGS %s
 #include "types.h"
 
 // LINKARGS: --wrap=None
-Option None() {
+Option None(void) {
     cr_log_info("returning `None`");
     Option none = {
         .value = 0,
diff --git a/tests/macro_attr/functions.c b/tests/macro_attr/functions.c
@@ -6,12 +6,12 @@ RUN: cat macro_attr_call_gates_1.ld | FileCheck --check-prefix=LINKARGS %s
 #include "functions.h"
 
 // LINKARGS: --wrap=f
-void f() {
+void f(void) {
     cr_log_info("Called `f()`");
 }
 
 // LINKARGS: --wrap=g
-void g() {
+void g(void) {
     cr_log_info("Called `g()`");
 }
 
@@ -22,16 +22,16 @@ void h(CB cb) {
 }
 
 // LINKARGS: --wrap=i
-void i() {
+void i(void) {
     cr_log_info("Called `i()`");
 }
 
 // LINKARGS: --wrap=j
-void j() {
+void j(void) {
     cr_log_info("Called `j()`");
 }
 
 // LINKARGS: --wrap=k
-void k() {
+void k(void) {
     cr_log_info("Called `k()`");
 }
diff --git a/tests/macro_attr/include/functions.h b/tests/macro_attr/include/functions.h
@@ -4,16 +4,16 @@
 #define EMPTY_FNLIKE(x)
 #define EMPTY_VARIADIC_FNLIKE(...)
 
-void f();
-ATTR void g();
+void f(void);
+ATTR void g(void);
 
 // CHECK: typedef struct IA2_fnptr__ZTSPFiiE CB;
 UNUSED typedef int (*CB)(int);
 
 void h(CB cb);
 
-EMPTY void i();
+EMPTY void i(void);
 
-EMPTY_FNLIKE(0) void j();
+EMPTY_FNLIKE(0) void j(void);
 
-EMPTY_VARIADIC_FNLIKE(1, 2) void k();
+EMPTY_VARIADIC_FNLIKE(1, 2) void k(void);
diff --git a/tests/minimal/include/minimal.h b/tests/minimal/include/minimal.h
@@ -1,10 +1,10 @@
 #pragma once
 
 // This function does nothing
-void foo();
+void foo(void);
 
 // This returns an integer
-int return_val();
+int return_val(void);
 
 // This takes an integer
 void arg1(int x);
diff --git a/tests/minimal/minimal.c b/tests/minimal/minimal.c
@@ -12,12 +12,12 @@ void arg1(int x) {
 }
 
 // LINKARGS: --wrap=foo
-void foo() {
+void foo(void) {
     cr_log_info("foo");
 }
 
 // LINKARGS: --wrap=return_val
-int return_val() {
+int return_val(void) {
     cr_log_info("return_val");
     return 1;
 }
diff --git a/tests/minimal_no_criterion/include/minimal.h b/tests/minimal_no_criterion/include/minimal.h
@@ -1,10 +1,10 @@
 #pragma once
 
 // This function does nothing
-void foo();
+void foo(void);
 
 // This returns an integer
-int return_val();
+int return_val(void);
 
 // This takes an integer
 void arg1(int x);
diff --git a/tests/minimal_no_criterion/main.c b/tests/minimal_no_criterion/main.c
@@ -12,7 +12,7 @@ INIT_RUNTIME(1);
 #define IA2_COMPARTMENT 1
 #include <ia2_compartment_init.inc>
 
-int main() {
+int main(void) {
     printf("Calling foo");
     foo();
 }
diff --git a/tests/minimal_no_criterion/minimal.c b/tests/minimal_no_criterion/minimal.c
@@ -11,12 +11,12 @@ void arg1(int x) {
 }
 
 // LINKARGS: --wrap=foo
-void foo() {
+void foo(void) {
     printf("foo");
 }
 
 // LINKARGS: --wrap=return_val
-int return_val() {
+int return_val(void) {
     printf("return_val");
     return 1;
 }
diff --git a/tests/rewrite_macros/include/lib.h b/tests/rewrite_macros/include/lib.h
@@ -35,5 +35,5 @@ extern struct event_actions actions;
 #define call_add_event(evt) IA2_CALL(actions.add, _ZTSPFbP5eventE)(evt)
 #endif
 
-struct event *get_event();
-void init_actions();
+struct event *get_event(void);
+void init_actions(void);
diff --git a/tests/rewrite_macros/lib.c b/tests/rewrite_macros/lib.c
@@ -16,7 +16,7 @@ struct event {
 };
 
 // LINKARGS: --wrap=get_event
-struct event *get_event() {
+struct event *get_event(void) {
     static struct event evt = { .id = 1 };
     return &evt;
 }
@@ -27,7 +27,7 @@ static bool nop(struct event *evt) {
 static void nop2(struct event *evt) { }
 
 // LINKARGS: --wrap=init_actions
-void init_actions() {
+void init_actions(void) {
     actions.add = nop;
     actions.del = nop;
     actions.enable = nop2;
diff --git a/tests/ro_sharing/include/plugin.h b/tests/ro_sharing/include/plugin.h
@@ -3,7 +3,7 @@
 #include <stdbool.h>
 #include <stdint.h>
 
-const char *get_plugin_str();
+const char *get_plugin_str(void);
 const uint32_t *get_plugin_uint(bool secret);
 void read_main_string(const char *str);
 void read_main_uint(const uint32_t *shared, const uint32_t *secret);
diff --git a/tests/ro_sharing/plugin.c b/tests/ro_sharing/plugin.c
@@ -23,7 +23,7 @@ const uint32_t plugin_shared_ro = 0x730283;
 uint32_t plugin_secret_rw = 0x8294671;
 
 // LINKARGS: --wrap=get_plugin_str
-const char *get_plugin_str() {
+const char *get_plugin_str(void) {
   return plugin_str;
 }
 
diff --git a/tests/should_segfault/include/print_secret.h b/tests/should_segfault/include/print_secret.h
@@ -3,6 +3,6 @@
 
 extern uint32_t secret;
 
-void print_secret();
+void print_secret(void);
 
-void do_early_fault();
+void do_early_fault(void);
diff --git a/tests/should_segfault/print_secret.c b/tests/should_segfault/print_secret.c
@@ -14,12 +14,12 @@ static bool early_fault = false;
 // Trigger a fault earlier than expected to test that CHECK_VIOLATION prints a
 // different message than in the mpk violation case.
 // LINKARGS: --wrap=do_early_fault
-void do_early_fault() {
+void do_early_fault(void) {
     early_fault = true;
 }
 
 // LINKARGS: --wrap=print_secret
-void print_secret() {
+void print_secret(void) {
     if (early_fault) {
         raise(SIGSEGV);
     }
diff --git a/tests/threads/include/library.h b/tests/threads/include/library.h
@@ -8,7 +8,7 @@ typedef void (*Fn)(void);
 
 // This function does nothing, but should get wrapped
 // LINKARGS: --wrap=library_foo
-void library_foo();
+void library_foo(void);
 
 // LINKARGS: --wrap=library_spawn_thread
 pthread_t library_spawn_thread(void);
diff --git a/tests/threads/library.c b/tests/threads/library.c
@@ -21,7 +21,7 @@ void library_call_fn(Fn what) {
 }
 
 // LINKARGS: --wrap=library_foo
-void library_foo() {
+void library_foo(void) {
   cr_log_info("data in library: %d\n", data_in_lib);
   cr_assert_eq(data_in_lib, 900);
 }
@@ -35,7 +35,7 @@ void library_memset(void *ptr, uint8_t byte, size_t n) {
 }
 
 // LINKARGS: --wrap=library_showpkru
-void library_showpkru() {
+void library_showpkru(void) {
   uint32_t actual_pkru = ia2_get_pkru();
   cr_log_info("library pkru %08x", actual_pkru);
   cr_assert_eq(0xfffffffc, actual_pkru);
diff --git a/tests/tls_protected/include/library.h b/tests/tls_protected/include/library.h
@@ -5,6 +5,6 @@
 thread_local extern uint32_t main_secret;
 thread_local extern uint32_t lib_secret;
 
-void lib_print_main_secret();
+void lib_print_main_secret(void);
 
-void lib_print_lib_secret();
+void lib_print_lib_secret(void);
diff --git a/tests/tls_protected/library.c b/tests/tls_protected/library.c
@@ -15,13 +15,13 @@
 
 thread_local uint32_t lib_secret = 0x1eaf1e55;
 
-void lib_print_main_secret() {
+void lib_print_main_secret(void) {
   cr_log_info("library: going to access main secret\n");
   cr_log_info("library: accessing main secret at %p\n", &main_secret);
   cr_log_info("library: main secret is %x\n", CHECK_VIOLATION(main_secret));
   cr_assert(false); // should not reach here
 }
 
-void lib_print_lib_secret() {
+void lib_print_lib_secret(void) {
   cr_log_info("library: lib secret is %x\n", lib_secret);
 }
diff --git a/tests/trusted_indirect/main.c b/tests/trusted_indirect/main.c
@@ -30,7 +30,7 @@ static uint32_t divide(uint32_t x, uint32_t y) {
     return x / y;
 }
 
-void call_fn_ptr() {
+void call_fn_ptr(void) {
     function_t f = get_function();
     cr_log_info("Got the function %s from the library\n", f.name);
     uint32_t x = 987234;
@@ -50,7 +50,7 @@ void call_fn_ptr() {
     cr_assert_eq(res3, 6);
 }
 
-void do_test() {
+void do_test(void) {
     // Test calling a function pointer with one of the shared library's functions
     call_fn_ptr();
 
diff --git a/tests/untrusted_indirect/foo.c b/tests/untrusted_indirect/foo.c
@@ -33,7 +33,7 @@ bool register_callback(callback_t cb) {
 }
 
 // LINKARGS: --wrap=unregister_callback
-void unregister_callback() {
+void unregister_callback(void) {
     function = pick_lhs;
     if (last_result) {
         if (!clean_exit) {
diff --git a/tests/untrusted_indirect/include/foo.h b/tests/untrusted_indirect/include/foo.h
diff --git a/tests/untrusted_indirect/main.c b/tests/untrusted_indirect/main.c

Original file line number	Diff line number	Diff line change
`@@ -52,6 +52,7 @@ function(add_ia2_compartment NAME TYPE)`
`52`	`52`	`set_target_properties(${NAME} PROPERTIES PKEY ${ARG_PKEY})`
`53`	`53`	`target_compile_options(${NAME} PRIVATE`
`54`	`54`	`"-Werror=incompatible-pointer-types"`
	`55`	`+ "-Werror=strict-prototypes"`
`55`	`56`	`"-fPIC"`
`56`	`57`	`)`
`57`	`58`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ char *allocate_stack(int i) {`
`36`	`36`	`return stack + STACK_SIZE - 8;`
`37`	`37`	`}`
`38`	`38`
`39`		`-void allocate_stack_0() {`
	`39`	`+void allocate_stack_0(void) {`
`40`	`40`	`ia2_stackptr_0[0] = allocate_stack(0);`
`41`	`41`	`}`
`42`	`42`
Original file line number	Diff line number	Diff line change
`@@ -12,12 +12,12 @@ void arg1(int x) {`
`12`	`12`	`}`
`13`	`13`
`14`	`14`	`// LINKARGS: --wrap=foo`
`15`		`-void foo() {`
	`15`	`+void foo(void) {`
`16`	`16`	`cr_log_info("foo");`
`17`	`17`	`}`
`18`	`18`
`19`	`19`	`// LINKARGS: --wrap=return_val`
`20`		`-int return_val() {`
	`20`	`+int return_val(void) {`
`21`	`21`	`cr_log_info("return_val");`
`22`	`22`	`return 1;`
`23`	`23`	`}`