On Thu, 22 Aug 2024, Bernd Edlinger wrote:

> This affects only the RISC-V targets, where the compiler options
> -gvariable-location-views and consequently also -ginline-points
> are disabled by default, which is unexpected and disables some
> useful features of the generated debug info.
> 
> Due to a bug in the gas assembler the .loc statement
> is not usable to generate location view debug info.
> That is detected by configure:
> 
> configure:31500: checking assembler for dwarf2 debug_view support
> configure:31509: .../riscv-unknown-elf/bin/as    -o conftest.o conftest.s >&5
> conftest.s: Assembler messages:
> conftest.s:5: Error: .uleb128 only supports constant or subtract expressions
> conftest.s:6: Error: .uleb128 only supports constant or subtract expressions
> configure:31512: $? = 1
> configure: failed program was
>         .file 1 "conftest.s"
>         .loc 1 3 0 view .LVU1
>         nop
>         .data
>         .uleb128 .LVU1
>         .uleb128 .LVU1
> 
> configure:31523: result: no
> 
> This results in dwarf2out_as_locview_support being set to false,
> and that creates a sequence of events, with the end result that
> most inlined functions either have no DW_AT_entry_pc, or one
> with a wrong entry pc value.
> 
> But the location views can also be generated without using any
> .loc statements, therefore we should enable the option
> -gvariable-location-views by default, regardless of the status
> of -gas-locview-support.
> 
> Note however, that the combination of the following compiler options
> -g -O2 -gvariable-location-views -gno-as-loc-support
> turned out to create invalid assembler intermediate files,
> with lots of assembler errors like:
> Error: leb128 operand is an undefined symbol: .LVU3
> 
> This affects all targets, except RISC-V of course ;-)
> and is fixed by the changes in dwarf2out.cc
> 
> Finally the .debug_loclists created without assembler locview support
> did contain location view pairs like v0000000ffffffff v000000000000000
> which is the value from FORCE_RESET_NEXT_VIEW, but that is most likely
> not as expected either, so change that as well.
> 
> 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.
> ---
>  gcc/dwarf2out.cc                            | 16 ++++++++++------
>  gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c |  3 +++
>  gcc/testsuite/gcc.dg/debug/dwarf2/inline6.c |  7 ++++++-
>  gcc/toplev.cc                               |  4 +---
>  4 files changed, 20 insertions(+), 10 deletions(-)
> 
> v2: fixed the boot-strap error triggered by v1 on any target, except RISC-V,
> the issue was triggered by libstdc++-v3/src/c++11/cxx11-ios_failure-lt.s
> which is generated using -gno-as-loc-support, which triggered a latent issue.
> 
> v3: added some tests for the fixed bugs.
> 
> v4: fixed test failures on some targets, avoid hash sign as it is target 
> dependent.
> 
> Regression-tested on x86_64-pc-linux-gnu, riscv-unknown-elf
> and riscv64-unknown-elf, OK for trunk?
> 
> diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc
> index 362d695674b..c30dad1513a 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)

I'm wondering about these hunks - dwarf2out_as_loc_support indicates
the assembler supports .loc and dwarf2out_as_locview_support
indicates it supports location views within .loc, it's odd to have
dwarf2out_as_locview_support set but not dwarf2out_as_loc_support.

How does this happen?  May I suggest to adjust
dwarf2out_default_as_locview_support to require
dwarf2out_default_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..6893ddfa2eb 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..f5ca02205c0 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)
>      {
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Reply via email to