Skip to content

Don't assume GCC toolchain supports the same dwarf format rust uses #1076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

mqudsi
Copy link
Contributor

@mqudsi mqudsi commented May 20, 2024

While it may be a fairly safe assumption that we can use the same dwarf version rust uses (0fff25a) if we're compiling with clang, it might not be a valid assumption for the Gnu toolchain.

I tried to pull the logic up to add_default_flags(), but that would require running is_flag_supported() in the call to add_default_flags() which, while possible, might not be the expected or desired behavior. The alternate implementation would look like this, but I think this PR as submitted is probably the better way to go.

diff --git a/src/lib.rs b/src/lib.rs
index 9b2816f..69b7e28 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1921,7 +1921,15 @@ impl Build {
                 cmd.args.push("-G".into());
             }
             let family = cmd.family;
-            family.add_debug_flags(cmd, self.get_dwarf_version());
+            let (flag, opt_flag) = family.get_debug_flags(self.get_dwarf_version());
+            if let Some(flag) = flag {
+                cmd.args.push(flag.into());
+            }
+            if let Some(flag) = opt_flag {
+                if self.is_flag_supported(&flag).unwrap_or(false) {
+                    cmd.args.push(flag.into());
+                }
+            }
         }
 
         if self.get_force_frame_pointer() {
diff --git a/src/tool.rs b/src/tool.rs
index ea0f36f..ff6f517 100644
--- a/src/tool.rs
+++ b/src/tool.rs
@@ -416,19 +416,21 @@ pub enum ToolFamily {
 }
 
 impl ToolFamily {
-    /// What the flag to request debug info for this family of tools look like
-    pub(crate) fn add_debug_flags(&self, cmd: &mut Tool, dwarf_version: Option) {
+    /// What the flag to request debug info for this family of tools look like.
+    ///
+    /// Return value is a tuple of `(Option, Option)` where `Flag` may
+    /// be set unconditionally but `FlagIfSupported` should be tested for support first.
+    pub(crate) fn get_debug_flags(
+        &self,
+        dwarf_version: Option,
+    ) -> (Option, Option) {
         match *self {
-            ToolFamily::Msvc { .. } => {
-                cmd.push_cc_arg("-Z7".into());
-            }
-            ToolFamily::Gnu | ToolFamily::Clang { .. } => {
-                cmd.push_cc_arg(
-                    dwarf_version
-                        .map_or_else(|| "-g".into(), |v| format!("-gdwarf-{}", v))
-                        .into(),
-                );
-            }
+            ToolFamily::Msvc { .. } => (Some("-Z7".into()), None),
+            ToolFamily::Clang { .. } => match dwarf_version {
+                Some(v) => (Some(format!("-gdwarf-{v}")), None),
+                None => (Some("-g".into()), None),
+            },
+            ToolFamily::Gnu => (Some("-g".into()), Some(format!("-g").into())),
         }
     }

Closes #1075.

While it may be a fairly safe assumption that we can use the same dwarf version
rust uses (0fff25a) if we're compiling with clang, it might not be a valid
assumption for the Gnu toolchain.

Closes rust-lang#1075.
@mqudsi
Copy link
Contributor Author

mqudsi commented May 20, 2024

I'm not sure what to do about the tests; they're hard-coded to expect -gdwarf-4 or -gdwarf-2 depending on the target but the tests don't surface gcc vs clang so it's hard to make them dynamic.

@mqudsi mqudsi closed this May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--gdwarf=4 is not compatible with some newer mainstream gcc/as toolchains
1 participant