Move debug-info and extra-debug-info out of the -Z flag #9770
Comments
|
I'm not sure it's quite there yet. The most obvious issue is that source code positions are not yet correctly set in LLVM IR in some very visible places (like if-expressions and function calls). This will easily (and understandably) lead to confusion among users (see #9641 for an example). Another issue is that activating debug info doesn't seem to be compatible with activating optimizations (i.e. the In any case, for the time being I guess we should at least print out a warning when debug info and optimization are turned on at the same time. |
|
I could certainly go through upgrading LLVM again, I'd just be shooting in the dark, though. I'll see if any of the commit messages look relevant. I know that @thestinger and others found this awesome tool in LLVM to narrow down bitcode files to something that we could report to LLVM, although perhaps it's just particular passes which interfere with debuginfo... Regardless, I don't think that we should promote this to a proper flag if we still cause lots of compilation problems with optimizations on. That being said, I'm perfectly OK with a warning being printed if you requested optimizations and debug info and then debug info is just not generated. |
|
@michaelwoerister: if you're hitting an assertion, you can have |
|
Maybe a less aggressive first step would be a configure flag that would make us attempt to build rustc with the |
|
Note that extra-debug-info implies debug-info, and might even be overwritten if the latter appears later in the command line. |
|
@alexcrichton Thanks for the offer but, as you say, updating LLVM is unlikely to solve the problem. Better not invest any time if it's just for this. @thestinger Thanks for the hint. I think the best course here is for me to spend a day or two to really dig into the assertion problem. Maybe I can allocate some time for that next week. For now I'll create an issue describing the problem. As far as moving debug info out of the |
|
A little update on this: After 2 days of digging into it I was able to locate the issue causing the LLVM assertion about metadata still being referenced after deletion. It's a memory management issue in LLVM where some objects are never freed in rare cases. There is already a bug report on the same issue in LLVM's bug tracker and I added some more details to it in case somebody is interested. How do we go about fixing issues in LLVM? Wait until they get fixed upstream? Fix them in our version? |
|
If the issues cause pain in our work and we have solutions we're confident in, we tend to land locally and work on upstreaming, I believe. |
|
Well, the fix I propose in the LLVM bug report will certainly get rid of the memory leak but I don't know enough about the inner workings of LLVM's debug info handling to know whether it would be better to solve the problem earlier. That is, instead of making the function resilient against duplicates in sub-program list, it would also make sense to dedup the list already earlier. I can try to make sure that we don't end up with duplicates in the first place (which, I think, is related to #7349). That would solve the problem on the Rust side and we would not have to wait on LLVM to fix the issue. |
|
Any updates on this? Perhaps this has baked long enough to make it to graduation? |
|
The issues mentioned earlier in this thread have all been solved. Also, thanks to @comex, there's also a preliminary debugging for Mac OS now. I think, I'd be OK with moving debuginfo to |
|
Adding E-easy and E-mentor, I'd be willing to mentor this. |
Move them all behind a new -C switch. This migrates some -Z flags and some top-level flags behind this -C codegen option. The -C flag takes values of the form "-C name=value" where the "=value" is optional for some flags. Flags affected: * --llvm-args => -C llvm-args * --passes => -C passes * --ar => -C ar * --linker => -C linker * --link-args => -C link-args * --target-cpu => -C target-cpu * --target-feature => -C target-fature * --android-cross-path => -C android-cross-path * --save-temps => -C save-temps * --no-rpath => -C no-rpath * -Z no-prepopulate => -C no-prepopulate-passes * -Z no-vectorize-loops => -C no-vectorize-loops * -Z no-vectorize-slp => -C no-vectorize-slp * -Z soft-float => -C soft-float * -Z gen-crate-map => -C gen-crate-map * -Z prefer-dynamic => -C prefer-dynamic * -Z no-integrated-as => -C no-integrated-as As a bonus, this also promotes the -Z extra-debug-info flag to a first class -g or --debuginfo flag. * -Z debug-info => removed * -Z extra-debug-info => -g or --debuginfo Closes rust-lang#9770 Closes rust-lang#12000
Move them all behind a new -C switch. This migrates some -Z flags and some top-level flags behind this -C codegen option. The -C flag takes values of the form "-C name=value" where the "=value" is optional for some flags. Flags affected: * --llvm-args => -C llvm-args * --passes => -C passes * --ar => -C ar * --linker => -C linker * --link-args => -C link-args * --target-cpu => -C target-cpu * --target-feature => -C target-fature * --android-cross-path => -C android-cross-path * --save-temps => -C save-temps * --no-rpath => -C no-rpath * -Z no-prepopulate => -C no-prepopulate-passes * -Z no-vectorize-loops => -C no-vectorize-loops * -Z no-vectorize-slp => -C no-vectorize-slp * -Z soft-float => -C soft-float * -Z gen-crate-map => -C gen-crate-map * -Z prefer-dynamic => -C prefer-dynamic * -Z no-integrated-as => -C no-integrated-as As a bonus, this also promotes the -Z extra-debug-info flag to a first class -g or --debuginfo flag. * -Z debug-info => removed * -Z extra-debug-info => -g or --debuginfo Closes #9770 Closes #12000
Move them all behind a new -C switch. This migrates some -Z flags and some top-level flags behind this -C codegen option. The -C flag takes values of the form "-C name=value" where the "=value" is optional for some flags. Flags affected: * --llvm-args => -C llvm-args * --passes => -C passes * --ar => -C ar * --linker => -C linker * --link-args => -C link-args * --target-cpu => -C target-cpu * --target-feature => -C target-fature * --android-cross-path => -C android-cross-path * --save-temps => -C save-temps * --no-rpath => -C no-rpath * -Z no-prepopulate => -C no-prepopulate-passes * -Z no-vectorize-loops => -C no-vectorize-loops * -Z no-vectorize-slp => -C no-vectorize-slp * -Z soft-float => -C soft-float * -Z gen-crate-map => -C gen-crate-map * -Z prefer-dynamic => -C prefer-dynamic * -Z no-integrated-as => -C no-integrated-as As a bonus, this also promotes the -Z extra-debug-info flag to a first class -g or --debuginfo flag. * -Z debug-info => removed * -Z extra-debug-info => -g or --debuginfo Closes rust-lang#9770 Closes rust-lang#12000
Move them all behind a new -C switch. This migrates some -Z flags and some top-level flags behind this -C codegen option. The -C flag takes values of the form "-C name=value" where the "=value" is optional for some flags. Flags affected: * --llvm-args => -C llvm-args * --passes => -C passes * --ar => -C ar * --linker => -C linker * --link-args => -C link-args * --target-cpu => -C target-cpu * --target-feature => -C target-fature * --android-cross-path => -C android-cross-path * --save-temps => -C save-temps * --no-rpath => -C no-rpath * -Z no-prepopulate => -C no-prepopulate-passes * -Z no-vectorize-loops => -C no-vectorize-loops * -Z no-vectorize-slp => -C no-vectorize-slp * -Z soft-float => -C soft-float * -Z gen-crate-map => -C gen-crate-map * -Z prefer-dynamic => -C prefer-dynamic * -Z no-integrated-as => -C no-integrated-as As a bonus, this also promotes the -Z extra-debug-info flag to a first class -g or --debuginfo flag. * -Z debug-info => removed * -Z extra-debug-info => -g or --debuginfo Closes #9770 Closes #12000
Move them all behind a new -C switch. This migrates some -Z flags and some top-level flags behind this -C codegen option. The -C flag takes values of the form "-C name=value" where the "=value" is optional for some flags. Flags affected: * --llvm-args => -C llvm-args * --passes => -C passes * --ar => -C ar * --linker => -C linker * --link-args => -C link-args * --target-cpu => -C target-cpu * --target-feature => -C target-fature * --android-cross-path => -C android-cross-path * --save-temps => -C save-temps * --no-rpath => -C no-rpath * -Z no-prepopulate => -C no-prepopulate-passes * -Z no-vectorize-loops => -C no-vectorize-loops * -Z no-vectorize-slp => -C no-vectorize-slp * -Z soft-float => -C soft-float * -Z gen-crate-map => -C gen-crate-map * -Z prefer-dynamic => -C prefer-dynamic * -Z no-integrated-as => -C no-integrated-as As a bonus, this also promotes the -Z extra-debug-info flag to a first class -g or --debuginfo flag. * -Z debug-info => removed * -Z extra-debug-info => -g or --debuginfo Closes #9770 Closes #12000
Add new lint [`misnamed-getters`] ``` changelog: Add new lint [`misnamed-getters`] ``` Closes rust-lang#9769 The current lint matches all methods with a body of just one expression under the form `(&mut?)? <expr>.field` where field doesn't match the name of the method but there is a field of the same type in `<expr>` that matches the name. This allows matching nested structs, for example for newtype wrappers. This may cast the net a bit too wide and cause false positives. I'll run [clippy_lint_tester](https://github.com/mikerite/clippy_lint_tester) on the top crates to see how frequently false positives happen. There also may be room for improvement by checking that the replacement field would work taking into account implementations of `Deref` and `DerefMut` even if the types don't exactly match but I don't know yet how this could be done.
Now that #9658 has landed, libstd is buildable with both
-Z debug-infoand-Z extra-debug-info. It might be time to graduate these flags out of the experimental-Zzone.The text was updated successfully, but these errors were encountered: