[Merged by Bors] - fix abi call in go

.github/workflows/ci.yaml

@@ -287,13 +287,22 @@ jobs:
       - run: pip3 install uvicorn[standard] # For http mirroring test with FastAPI.
       - uses: actions/setup-go@v3
         with:
-          go-version: "1.19.0"
+          go-version: "1.19.5"
       - run: |
           cd mirrord/layer/tests/apps/self_open
           go build -o 19
       - run: |
           cd mirrord/layer/tests/apps/fileops/go
           go build -o fileops        
+      - run: |
+          cd mirrord/layer/tests/apps/issue834
+          go build -o 19
+      - uses: actions/setup-go@v3
+        with:
+          go-version: "1.18.10"
+      - run: |
+          cd mirrord/layer/tests/apps/issue834
+          go build -o 18
       - run: |
           cd mirrord/layer/tests/apps/fileops
           cargo build

CHANGELOG.md

@@ -7,6 +7,10 @@ Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how
 
 ## [Unreleased]
 
+### Fixed
+
+- Go crash in some scenarios [#834](https://github.com/metalbear-co/mirrord/issues/834).
+
 ## 3.19.2
 
 ### Changed

mirrord/layer/src/go_hooks.rs

@@ -525,10 +525,14 @@ unsafe extern "C" fn go_syscall_new_detour() {
         "mov r13, rsi",
         // save rcx in r15
         "mov r15, rcx",
-        "call enter_syscall",
         // Save stack
         "mov rdx, rsp",
-        "mov rdi, qword ptr FS:[0xfffffff8]",
+        // in the past, we tried to retrieve g from fs:[-0x8]
+        // but this sometimes fails to get g for some reason
+        // and r14 seems to be more correct
+        "mov rdi, r14",
+        // for any case, store it there in case it isn't stored
+        "mov qword ptr fs:[0xfffffff8], rdi",
         "cmp rdi, 0x0",
         "jz 1f",
         "mov rax, qword ptr [rdi + 0x30]",
@@ -562,10 +566,6 @@ unsafe extern "C" fn go_syscall_new_detour() {
         "sub rsi, qword ptr [ rsp + 0x28]",
         "mov qword ptr fs:[0xfffffff8], rdi",
         "mov rsp, rsi",
-        // exit syscall - it clobbers rax so we need to save it
-        "mov rbx, rax",
-        "call exit_syscall",
-        "mov rax, rbx",
         // Regular flow
         "cmp    rax, -0xfff",
         "jbe    2f",
@@ -584,21 +584,17 @@ unsafe extern "C" fn go_syscall_new_detour() {
         "mov    QWORD PTR [rsp+0x30], 0x0",
         "mov    QWORD PTR [rsp+0x28], rdx",
         // Call ABI handler
-        "mov QWORD PTR [rsp], r9",
+        "mov QWORD PTR [rsp], r11",
         "mov r9, r8",
-        "mov r8, rsi",
+        "mov r8, r13",
         "mov rsi, rbx",
-        "mov rdx, rcx",
+        "mov rdx, r15",
         "mov rcx, r10",
-        "mov rdi, rax",
+        "mov rdi, r12",
         "call c_abi_syscall6_handler",
         // restore
         "mov    rsi, QWORD PTR [rsp+0x28]",
         "mov    rsp, rsi",
-        // exit syscall - it clobbers rax so we need to save it
-        "mov rbx, rax",
-        "call exit_syscall",
-        "mov rax, rbx",
         // Regular flow
         "cmp    rax, -0xfff",
         "jbe    2f",

mirrord/layer/tests/apps/issue834/go.mod

@@ -0,0 +1,8 @@
+module app_go
+
+go 1.18
+
+require (
+	go.kuoruan.net/v8go-polyfills v0.5.0
+	rogchap.com/v8go v0.7.0
+)

mirrord/layer/tests/apps/issue834/go.sum

@@ -0,0 +1,4 @@
+go.kuoruan.net/v8go-polyfills v0.5.0 h1:wd2WxsFIXWK/FcrpITw6BOo8Rn24xMmd4qoHofgg8hc=
+go.kuoruan.net/v8go-polyfills v0.5.0/go.mod h1:egHzK8RIHR7dPOYzhnRsomClFTVmYCtvhTWqec4JXaY=
+rogchap.com/v8go v0.7.0 h1:kgjbiO4zE5itA962ze6Hqmbs4HgZbGzmueCXsZtremg=
+rogchap.com/v8go v0.7.0/go.mod h1:MxgP3pL2MW4dpme/72QRs8sgNMmM0pRc8DPhcuLWPAs=

mirrord/layer/tests/apps/issue834/main.go

@@ -0,0 +1,12 @@
+package main
+
+import (
+	"fmt"
+
+	_ "go.kuoruan.net/v8go-polyfills"
+	_ "rogchap.com/v8go"
+)
+
+func main() {
+	fmt.Println("okay")
+}

mirrord/layer/tests/common/mod.rs

@@ -343,6 +343,8 @@ pub enum Application {
     EnvBashCat,
     NodeFileOps,
     GoDir,
+    Go19Issue834,
+    Go18Issue834,
 }
 
 impl Application {
@@ -385,6 +387,8 @@ impl Application {
             Application::EnvBashCat => String::from("tests/apps/env_bash_cat.sh"),
             Application::NodeFileOps => String::from("node"),
             Application::GoDir => String::from("tests/apps/dir_go/dir_go"),
+            Application::Go19Issue834 => String::from("tests/apps/issue834/19"),
+            Application::Go18Issue834 => String::from("tests/apps/issue834/18"),
         }
     }
 
@@ -423,6 +427,8 @@ impl Application {
             Application::Go19HTTP => vec![],
             Application::GoDir => vec![],
             Application::GoFileOps => vec![],
+            Application::Go19Issue834 => vec![],
+            Application::Go18Issue834 => vec![],
             Application::RustFileOps => vec![],
             Application::EnvBashCat => vec![],
         }
@@ -439,6 +445,8 @@ impl Application {
             | Application::RustFileOps
             | Application::EnvBashCat
             | Application::NodeFileOps
+            | Application::Go19Issue834
+            | Application::Go18Issue834
             | Application::GoDir => {
                 unimplemented!("shouldn't get here")
             }

mirrord/layer/tests/issue834.rs

@@ -0,0 +1,41 @@
+#[cfg(target_os = "linux")]
+use std::{path::PathBuf, time::Duration};
+
+#[cfg(target_os = "linux")]
+use rstest::rstest;
+#[cfg(target_os = "linux")]
+use tokio::net::TcpListener;
+
+#[cfg(target_os = "linux")]
+mod common;
+
+#[cfg(target_os = "linux")]
+pub use common::*;
+
+/// Verify that issue [#834](https://github.com/metalbear-co/mirrord/issues/834) is fixed
+#[cfg(target_os = "linux")]
+#[rstest]
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+#[timeout(Duration::from_secs(60))]
+async fn test_issue834(
+    #[values(Application::Go18Issue834, Application::Go19Issue834)] application: Application,
+    dylib_path: &PathBuf,
+) {
+    let executable = application.get_executable().await; // Own it.
+    println!("Using executable: {}", &executable);
+    let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
+    let addr = listener.local_addr().unwrap().to_string();
+    println!("Listening for messages from the layer on {addr}");
+    let env = get_env(dylib_path.to_str().unwrap(), &addr);
+    let mut test_process =
+        TestProcess::start_process(executable, application.get_args(), env).await;
+
+    // Accept the connection from the layer and verify initial messages.
+    let mut _layer_connection = LayerConnection::get_initialized_connection(&listener).await;
+
+    test_process.wait().await;
+    test_process.assert_stdout_contains("okay");
+
+    test_process.assert_no_error_in_stdout();
+    test_process.assert_no_error_in_stderr();
+}