rtld: Add missing "memory" clobbers to prevent reordering

I was only testing a -O0 rtld so the inline assembly happened to work.
However, at -O2 the compiler is free to reorder instruction before/after
the inline asm (including the restore of $cgp after a function call).
This was causing the rtld $cgp to be re-installed after the inline asm so
the function calls would then crash due to a wrong $cgp.

Using "memory" clobbers should fix this issue but for the two cases the
return the target function pointer it would be much nicer if we could just
return "target+cgp" in $c3/$c4 instead of having to use an on-stack output
argument.

See CTSRD-CHERI/llvm-project#326

libexec/rtld-cheri-elf/mips/cheri_plt.cpp

@@ -244,7 +244,10 @@ _mips_rtld_bind(void* _plt_stub)
 	lock_release(rtld_bind_lock, &lockstate);
 	// Setup the target $cgp so that we can actually call the function
 	// TODO: return two values instead (will a 2 cap struct use $c3/$c4?)
-	__asm__ volatile("cmove $cgp, %0"::"C"(target_cgp));
+	// FIXME: memory clobber should ensure that this is moved after the
+	//  restore of $cgp (due to the call to lock_release) but it would be
+	//  much nicer if we could just return (target, $cgp) in $c3,$c4
+	__asm__ volatile("cmove $cgp, %0"::"C"(target_cgp): "$c26", "memory");
 	return target;
 }
 

libexec/rtld-cheri-elf/mips/rtld_machdep.h

@@ -41,6 +41,7 @@
 #else
 #include <assert.h>
 #define dbg_assert(cond) assert(cond)
+#define dbg_cheri(...)
 #define dbg(...)
 #endif
 
@@ -241,10 +242,26 @@ _call_init_fini_array_pointer(const struct Struct_Obj_Entry *obj, vaddr_t target
 	/* Set the target object $cgp when calling the pointer:
 	 * Note: we use target_cgp_for_func() to support per-function captable */
 	const void *init_fini_cgp = target_cgp_for_func(obj, (dlfunc_t)func);
-	__asm__ volatile("cmove $cgp, %0" :: "C"(init_fini_cgp));
-#endif
-	/* FIXME: should probably do this from assembly to ensure that $cgp is correct */
+	dbg_cheri("Setting init function $cgp to %#p for call to %#p. "
+	    "Current $cgp: %#p\n", init_fini_cgp, (void*)func, cheri_getidc());
+	/*
+	 * Invoke the function from assembly to ensure that the $cgp value is
+	 * correct and the compiler can't reorder things.
+	 * When I was calling the function from C, it broke at -O2.
+	 * Note: we need the memory clobber here to ensure that the setting of
+	 * $cgp is not ignored due to reordering of instructions (e.g. by adding
+	 * a $cgp restore after external function calls).
+	 */
+	__asm__ volatile("cmove $cgp, %[cgp_val]\n\t"
+	    :/*out*/
+	    :/*in*/[cgp_val]"C"(init_fini_cgp)
+	    :/*clobber*/"$c26", "memory");
+	func(argc, argv, env);
+	/* Ensure that the function call is not reordered before/after asm */
+	__compiler_membar();
+#else
 	func(argc, argv, env);
+#endif
 }
 
 #define call_init_array_pointer(obj, target)			\

libexec/rtld-elf/rtld.c

@@ -887,8 +887,12 @@ _rtld(Elf_Addr *sp, func_ptr_type *exit_proc, Obj_Entry **objp)
     // and should have the same effect.
     const void *entry_cgp = target_cgp_for_func(obj_main, (dlfunc_t)obj_main->entry);
     dbg_cheri("Setting initial $cgp for %s to %-#p", obj_main->path, entry_cgp);
-    __asm__ volatile("cmove $cgp, %0" :: "C"(entry_cgp));
     assert(cheri_getperm(obj_main->entry) & CHERI_PERM_EXECUTE);
+    /* Add memory clobber to ensure that it is done last thing before return
+     *
+     * TODO: would be nice if we could return pairs in $c3/$c4
+     */
+    __asm__ volatile("cmove $cgp, %0" :: "C"(entry_cgp): "$c26", "memory");
 #endif
     return (func_ptr_type) obj_main->entry;
 }