Message ID | AS1PR01MB9465CA59822CED782BA56D23E48E2@AS1PR01MB9465.eurprd01.prod.exchangelabs.com |
---|---|
State | New |
Headers | show |
Series | [v3] RISC-V: Enable -gvariable-location-views by default | expand |
Hello, Bernd, Thanks for the fixes and improvements, your patch looks good to me. I stand behind its approval by someone with authority to do so. I believe location views are somewhat problematic on RISC-V, because of the object code relaxations. Its usefulness is also limited without support from debug consumers. It's unfortunate, in hindsight, that inline entry points ended up tied to location views. I can't recall any strong reason for that. I suppose lifting that requirement, so as to enable inline points even without location views, would be desirable. Would you be interested in pursuing that? On Aug 21, 2024, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > gcc/ChangeLog: > * dwarf2out.cc (dwarf2out_maybe_output_loclist_view_pair, > output_loc_list): Correct handling of -gno-as-loc-support, > use ZERO_VIEW_P to output view number as zero value. > * toplev.cc (process_options): Do not automatically disable > -gvariable-location-views when -gno-as-loc-support or > -gno-as-locview-support is used. > gcc/testsuite/ChangeLog: > * gcc.dg/debug/dwarf2/inline2.c: Add checks for inline entry_pc. > * gcc.dg/debug/dwarf2/inline6.c: Add -gno-as-loc-support and check > the resulting location views.
Hello Alexandre, On 8/22/24 05:39, Alexandre Oliva wrote: > Hello, Bernd, > > Thanks for the fixes and improvements, your patch looks good to me. I > stand behind its approval by someone with authority to do so. > > I believe location views are somewhat problematic on RISC-V, because of > the object code relaxations. Its usefulness is also limited without > support from debug consumers. > The main deficiency with the location view line-numbers I see, is that it is possible to have different line numbers on the same PC, which have different view numbers, but when debugging an inlined function, it is sometimes impossible to tell if the line number is from the inline function or from the calling function, especially at the end of an inline code range, that is because the DW_AT_ranges value of the inlined_subroutine has only begin and end PC, but to make it clear the begin and end PC+View number would be necessary. This means it is ambiguous, and that probably stated with the non-is-stmt line numbers. > It's unfortunate, in hindsight, that inline entry points ended up tied > to location views. I can't recall any strong reason for that. I > suppose lifting that requirement, so as to enable inline points even > without location views, would be desirable. Would you be interested in > pursuing that? > Thanks a lot for your support, yes, that sounds reasonable, I can look into breaking this connection. PS: I need to make a small fixup to the test cases, as the linaro folks indicated that my test cases should not include the comment sign in "# DW_AT_entry_pc", as that is target dependent. Thanks Bernd. > On Aug 21, 2024, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > >> gcc/ChangeLog: > >> * dwarf2out.cc (dwarf2out_maybe_output_loclist_view_pair, >> output_loc_list): Correct handling of -gno-as-loc-support, >> use ZERO_VIEW_P to output view number as zero value. >> * toplev.cc (process_options): Do not automatically disable >> -gvariable-location-views when -gno-as-loc-support or >> -gno-as-locview-support is used. > >> gcc/testsuite/ChangeLog: > >> * gcc.dg/debug/dwarf2/inline2.c: Add checks for inline entry_pc. >> * gcc.dg/debug/dwarf2/inline6.c: Add -gno-as-loc-support and check >> the resulting location views. >
diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc index 346feeb53c8..79f97b5a55e 100644 --- a/gcc/dwarf2out.cc +++ b/gcc/dwarf2out.cc @@ -10374,7 +10374,7 @@ dwarf2out_maybe_output_loclist_view_pair (dw_loc_list_ref curr) #ifdef DW_LLE_view_pair dw2_asm_output_data (1, DW_LLE_view_pair, "DW_LLE_view_pair"); - if (dwarf2out_as_locview_support) + if (dwarf2out_as_locview_support && dwarf2out_as_loc_support) { if (ZERO_VIEW_P (curr->vbegin)) dw2_asm_output_data_uleb128 (0, "Location view begin"); @@ -10396,8 +10396,10 @@ dwarf2out_maybe_output_loclist_view_pair (dw_loc_list_ref curr) } else { - dw2_asm_output_data_uleb128 (curr->vbegin, "Location view begin"); - dw2_asm_output_data_uleb128 (curr->vend, "Location view end"); + dw2_asm_output_data_uleb128 (ZERO_VIEW_P (curr->vbegin) + ? 0 : curr->vbegin, "Location view begin"); + dw2_asm_output_data_uleb128 (ZERO_VIEW_P (curr->vend) + ? 0 : curr->vend, "Location view end"); } #endif /* DW_LLE_view_pair */ @@ -10430,7 +10432,7 @@ output_loc_list (dw_loc_list_ref list_head) vcount++; /* ?? dwarf_split_debug_info? */ - if (dwarf2out_as_locview_support) + if (dwarf2out_as_locview_support && dwarf2out_as_loc_support) { char label[MAX_ARTIFICIAL_LABEL_BYTES]; @@ -10460,10 +10462,12 @@ output_loc_list (dw_loc_list_ref list_head) } else { - dw2_asm_output_data_uleb128 (curr->vbegin, + dw2_asm_output_data_uleb128 (ZERO_VIEW_P (curr->vbegin) + ? 0 : curr->vbegin, "View list begin (%s)", list_head->vl_symbol); - dw2_asm_output_data_uleb128 (curr->vend, + dw2_asm_output_data_uleb128 (ZERO_VIEW_P (curr->vend) + ? 0 : curr->vend, "View list end (%s)", list_head->vl_symbol); } diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c b/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c index 9c36450ae2d..03d1a73c0c4 100644 --- a/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c @@ -30,6 +30,9 @@ layout. */ /* { dg-final { scan-assembler-times "\\(DIE \\(\[^\n\]*\\) DW_TAG_lexical_block" 0 } } */ +/* Each inline instance should have DW_AT_entry_pc and DW_AT_GNU_entry_view. */ +/* { dg-final { scan-assembler-times "# DW_AT_entry_pc" 6 } } */ +/* { dg-final { scan-assembler-times "# DW_AT_GNU_entry_view" 6 } } */ /* There are 3 DW_AT_inline attributes: one per abstract inline instance. The value of the attribute must be 0x3, meaning the function was diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/inline6.c b/gcc/testsuite/gcc.dg/debug/dwarf2/inline6.c index fde8c27820d..a3b0e6dac18 100644 --- a/gcc/testsuite/gcc.dg/debug/dwarf2/inline6.c +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/inline6.c @@ -16,7 +16,7 @@ /* Explicitly use dwarf-5 which uses DW_FORM_implicit_const. */ /* { dg-do compile } */ -/* { dg-options "-O -g3 -gdwarf-5 -dA -fgnu89-inline" } */ +/* { dg-options "-O -g3 -gdwarf-5 -dA -fgnu89-inline -gno-as-loc-support" } */ /* There are 6 inlined subroutines: - One for each subroutine inlined into main, that's 3. @@ -29,6 +29,11 @@ layout. */ /* { dg-final { scan-assembler-times "\\(DIE \\(\[^\n\]*\\) DW_TAG_lexical_block" 0 } } */ +/* Each inline instance should have DW_AT_entry_pc and DW_AT_GNU_entry_view. */ +/* { dg-final { scan-assembler-times "# DW_AT_entry_pc" 6 } } */ +/* { dg-final { scan-assembler-times "# DW_AT_GNU_entry_view" 6 } } */ +/* { dg-final { scan-assembler-times "uleb128\[^\n\]*LVU\[^\n\]*# View list (begin|end)" 0 } } */ +/* { dg-final { scan-assembler-times "uleb128\[^\n\]*0xffffffff\[^\n\]*# View list (begin|end)" 0 } } */ /* There are 3 DW_AT_inline attributes: one per abstract inline instance. The value of the attribute must be 0x3, meaning the function was diff --git a/gcc/toplev.cc b/gcc/toplev.cc index eee4805b504..292948122de 100644 --- a/gcc/toplev.cc +++ b/gcc/toplev.cc @@ -1475,9 +1475,7 @@ process_options () = (flag_var_tracking && debug_info_level >= DINFO_LEVEL_NORMAL && dwarf_debuginfo_p () - && !dwarf_strict - && dwarf2out_as_loc_support - && dwarf2out_as_locview_support); + && !dwarf_strict); } else if (debug_variable_location_views == -1 && dwarf_version != 5) {