From 71e478d251708113d14d42911653e2899f3f5c84 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= <pablob@amazon.com>
Date: Mon, 9 Sep 2024 15:05:57 +0200
Subject: [PATCH] fix(release): ensure debuginfo file contains debugging info
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Seems recently Cargo defaulted to use `strip=debuginfo`. This
inadvertently made our debuginfo files much smaller

Fix the issue by using `strip=none` and add a test so that it breaks if
this somehow changes again.

Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
---
 CHANGELOG.md                                  |  3 ++
 Cargo.toml                                    |  4 ++
 .../functional/test_binary.py                 | 48 +++++++++++++++++++
 .../functional/test_binary_static_linking.py  | 22 ---------
 4 files changed, 55 insertions(+), 22 deletions(-)
 create mode 100644 tests/integration_tests/functional/test_binary.py
 delete mode 100644 tests/integration_tests/functional/test_binary_static_linking.py

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e0ad7ca78fd..d5232f0db1a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,9 @@ and this project adheres to
   from net device creation to device activation time, but it was not reflected
   in the restore path. This was leading to inability to connect to the restored
   VM if the offload features were used.
+- [#4829](https://github.com/firecracker-microvm/firecracker/pull/4829): v1.9.0
+  was missing most of the debugging information in the debuginfo file, due to a
+  change in the Cargo defaults. This has been corrected.
 
 ## \[1.9.0\]
 
diff --git a/Cargo.toml b/Cargo.toml
index 1a5187f4f2f..914c4cdc87c 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -27,3 +27,7 @@ panic = "abort"
 [profile.release]
 panic = "abort"
 lto = true
+strip = "none"
+
+[profile.bench]
+strip = "debuginfo"
diff --git a/tests/integration_tests/functional/test_binary.py b/tests/integration_tests/functional/test_binary.py
new file mode 100644
index 00000000000..67181bd3edb
--- /dev/null
+++ b/tests/integration_tests/functional/test_binary.py
@@ -0,0 +1,48 @@
+# Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# SPDX-License-Identifier: Apache-2.0
+
+"""Tests to check several aspects of the binaries"""
+
+import re
+import subprocess
+
+import pytest
+
+from framework import utils
+
+
+@pytest.mark.timeout(500)
+def test_firecracker_binary_static_linking(microvm_factory):
+    """
+    Test to make sure the firecracker binary is statically linked.
+    """
+    fc_binary_path = microvm_factory.fc_binary_path
+    _, stdout, stderr = utils.check_output(f"file {fc_binary_path}")
+    assert "" in stderr
+    # expected "statically linked" for aarch64 and
+    # "static-pie linked" for x86_64
+    assert "statically linked" in stdout or "static-pie linked" in stdout
+
+
+def test_release_debuginfo(microvm_factory):
+    """Ensure the debuginfo file has the right ELF sections"""
+    fc_binary = microvm_factory.fc_binary_path
+    debuginfo = fc_binary.with_suffix(".debug")
+    stdout = subprocess.check_output(
+        ["readelf", "-S", str(debuginfo)],
+        encoding="ascii",
+    )
+    matches = {
+        match[0] for match in re.findall(r"\[..] (\.(\w|\.)+)", stdout, re.MULTILINE)
+    }
+    needed_sections = {
+        ".debug_aranges",
+        ".debug_info",
+        ".debug_abbrev",
+        ".debug_line",
+        ".debug_frame",
+        ".debug_str",
+        ".debug_ranges",
+    }
+    missing_sections = needed_sections - matches
+    assert missing_sections == set()
diff --git a/tests/integration_tests/functional/test_binary_static_linking.py b/tests/integration_tests/functional/test_binary_static_linking.py
deleted file mode 100644
index 1c277a4d00b..00000000000
--- a/tests/integration_tests/functional/test_binary_static_linking.py
+++ /dev/null
@@ -1,22 +0,0 @@
-# Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
-# SPDX-License-Identifier: Apache-2.0
-"""Tests to check if the release binary is statically linked.
-
-"""
-
-import pytest
-
-from framework import utils
-
-
-@pytest.mark.timeout(500)
-def test_firecracker_binary_static_linking(microvm_factory):
-    """
-    Test to make sure the firecracker binary is statically linked.
-    """
-    fc_binary_path = microvm_factory.fc_binary_path
-    _, stdout, stderr = utils.check_output(f"file {fc_binary_path}")
-    assert "" in stderr
-    # expected "statically linked" for aarch64 and
-    # "static-pie linked" for x86_64
-    assert "statically linked" in stdout or "static-pie linked" in stdout