Skip to content
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

-Cdebuginfo=1 wastefully produces full type descriptions. #69074

Closed
eddyb opened this issue Feb 11, 2020 · 10 comments
Closed

-Cdebuginfo=1 wastefully produces full type descriptions. #69074

eddyb opened this issue Feb 11, 2020 · 10 comments
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) C-bug Category: This is a bug. I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Feb 11, 2020

This example makes -Cdebuginfo=1 emit the variant (see godbolt output):

pub enum Foo {
    ThisShouldNotBeInDebugInfo([u8; 1337])
}

impl Foo {
    pub fn foo() {}
}

Its resulting LLVM IR shows all of these DebugInfo metadata nodes:

!6 = !DICompositeType(tag: DW_TAG_structure_type, name: "Foo", scope: !8, file: !7, size: 10696, align: 8, elements: !{!10}, identifier: "65d4d9f1044eb0325dc66254af8523df")
!10 = !DICompositeType(tag: DW_TAG_variant_part, scope: !8, file: !7, size: 10696, align: 8, elements: !{!12}, templateParams: !4, identifier: "65d4d9f1044eb0325dc66254af8523df_variant_part")
!12 = !DIDerivedType(tag: DW_TAG_member, name: "ThisShouldNotBeInDebugInfo", scope: !10, file: !7, baseType: !13, size: 10696, align: 8)
!13 = !DICompositeType(tag: DW_TAG_structure_type, name: "ThisShouldNotBeInDebugInfo", scope: !6, file: !7, size: 10696, align: 8, elements: !{!15}, templateParams: !4, identifier: "65d4d9f1044eb0325dc66254af8523df::ThisShouldNotBeInDebugInfo")
!15 = !DIDerivedType(tag: DW_TAG_member, name: "__0", scope: !13, file: !7, baseType: !16, size: 10696, align: 8)
!16 = !DICompositeType(tag: DW_TAG_array_type, baseType: !17, size: 10696, align: 8, elements: !{!19})
!17 = !DIBasicType(name: "u8", size: 8, encoding: DW_ATE_unsigned)
!19 = !DISubrange(count: 1337)

But at -Cdebuginfo=1, this is really what we want to see:

!6 = !DICompositeType(tag: DW_TAG_structure_type, name: "Foo", scope: !8, file: !7)

If this doesn't work we can always just use DINamespace instead.


IIUC, this specific example is caused from the way we handle inherent impls, but there might be others, so we probably want to make the debuginfo generating infrastructure ICE when it's trying to generate non-trivial type debuginfo but -Cdebuginfo=1 is set.

Oh, and, I found this because librustc_driver-*.so is 1GB with debuginfo-level=1 (and debug-assertions=1) in config.toml, and most of that is type debuginfo.

cc @rust-lang/compiler @alexcrichton

@jonas-schievink jonas-schievink added A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Feb 11, 2020
@eddyb
Copy link
Member Author

eddyb commented Feb 11, 2020

More examples (using enum Foo from above):

pub static FOO: Option<Foo> = None;
pub fn foo() -> &'static dyn Sync {
    &None::<Foo>
}

However, clang generates no debuginfo for this at -g1 (see godbolt output):

struct {
    int should_not_be_in_debug_info;
} foo;

I think we should follow clang -g1's example for rustc -Cdebuginfo=1.

@michaelwoerister
Copy link
Member

I think this is a recent regression. Do we know when it was introduced?

A few months ago when I filed #64405, this wasn't the case yet.

@eddyb
Copy link
Member Author

eddyb commented Feb 12, 2020

Ah, weird, it looked like it's been like this forever, thankfully godbolt has all stable versions so we can narrow it down quickly.

In that issue you suggest we should not even be emitting DISubprogram, will that still let inlined calls show up on the backtrace? I guess I can cause an ICE on purpose to find out.

It would be good to remove even more than #69080 does right now, because then we might stand a chance to ship debuginfo.

Of course the ideal thing would be split DWARF and a rustc-debuginfo component in rustup but I am not sure how to get there.

@bjorn3
Copy link
Member

bjorn3 commented Feb 12, 2020

In that issue you suggest we should not even be emitting DISubprogram, will that still let inlined calls show up on the backtrace?

Inlined calls are determined from DW_TAG_inlined_subroutine inside the respective DW_TAG_subroutine I believe.

@bjorn3
Copy link
Member

bjorn3 commented Feb 12, 2020

Of course the ideal thing would be split DWARF and a rustc-debuginfo component in rustup but I am not sure how to get there.

That would reduce the quality of ICE backtraces reported by the average user.

@eddyb
Copy link
Member Author

eddyb commented Feb 12, 2020

That would reduce the quality of ICE backtraces reported by the average user.

Right now we have no debuginfo at all AFAIK, but if there was an optional component we could tell people to install it. Or maybe we could take the backtrace they report, and the version, and generate the full backtrace ourselves using the right debuginfo (since all you need is addresses, right?).

@bjorn3
Copy link
Member

bjorn3 commented Feb 12, 2020

Right now we have no debuginfo at all AFAIK

Forgot that :)

since all you need is addresses, right?

And ASLR bias I think.

Or maybe we could take the backtrace they report, and the version, and generate the full backtrace ourselves using the right debuginfo

It would be nice if a bot could do it.

@petrochenkov
Copy link
Contributor

ping in #69074 (comment)

[triagebot] The librustc_driver size reduction looks great, the behavior seems in line with what -Cdebuginfo=1 documents, no idea about the implementation details.

@eddyb
Copy link
Member Author

eddyb commented Mar 17, 2020

I've decided, in the interest of getting something merged, to not try to do everything in #69080.

Which leaves out:

  • moving the type-related fields in CrateDebugContext (created_enum_disr_types and composite_types_completed) into TypeMap, and adding an Option around RefCell<TypeMap<'a, 'tcx>>, to make it impossible to generate type debuginfo unless the debuginfo level is 2 (so instead of a repeat of this bug, we'd get an ICE)
  • looking into not generating DISubprograms for -Cdebuginfo=1 (-Cdebuginfo=1 wastefully produces full type descriptions. #69074 (comment))
    • this might heavily degrade debuginfo for inlined functions, so maybe we can "just" try to make DISubprogram cheaper instead somehow?

Centril added a commit to Centril/rust that referenced this issue Mar 23, 2020
…-a-bar, r=michaelwoerister

rustc_codegen_llvm: don't generate any type debuginfo for -Cdebuginfo=1.

Works towards rust-lang#69074 by adding more checks for `DebugInfo::Full` in a few places in `rustc_codegen_llvm`, bringing us in line with what `clang -g1` generates (no debuginfo types, nor debuginfo for `static`s).

<hr/>

My local build's (`debuginfo-level=1`, `debug-assertions=1`) `librustc_driver-*.so` went from just over 1GiB (1019MiB) down to 402MiB.

It's still bad, but the `.debug_*` sections themselves (as reported by `objdump`) went from something like 853MiB down to 236MiB, i.e. roughly a 3.6x reduction.

<hr/>

Sadly, I don't think this is enough to justify *shipping* all of this debuginfo, but now it's more plausible that we could at least *build* with `debuginfo-level=1` *then* strip it.
That would give us real backtraces for e.g. ICEs during builds, but I don't know how often that's relevant.

We could also look into split DWARF, and maybe have a `rustc-debuginfo` component in `rustup`.

There's also the possibility of making it slimmer by omitting parameters to functions, or perhaps some deduplication (I think right now there is no DWARF reuse across CGUs? maybe ThinLTO helps?).

r? @michaelwoerister cc @rust-lang/wg-codegen @alexcrichton @Mark-Simulacrum
@wesleywiser
Copy link
Member

Visited during wg-debugging triage. There are various issues with changing what debuginfo is generated by -Cdebuginfo=1 such as #64405. #109808 introduced new, extended flags for -Cdebuginfo which allow you to only generate line tables which seems to be the original intent of -Cdebuginfo=1 anyway. Closing as completed by that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) C-bug Category: This is a bug. I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants