RE: [PATCH 00/30] Allow location description on the DWARF stack

["Tye, Tony" <Tony.Tye@amd.com>]

Hi Markus,
Care was taken to define things in a backwards compatible way so that the existing semantics of DWARF 5 are not changed. All existing operators still behave the same way when given the same arguments. This allows all the existing gdb tests to continue to pass with these changes. So it is optional whether a producer uses the extra capabilities of the extension. Effectively the extension is relaxing restrictions in DWARF 5 to allow more expressions to become legal. It is not changing the meaning of any existing expressions.

The extension suggests using the existing augmentation fields to indicate if it is being used, however there are other extensions supported by gdb that do not have any indication besides using new operators.

It would be good to know if this helps address the issues you have been encountering.

Thanks,
-Tony

-----Original Message-----
From: Gdb-patches <gdb-patches-bounces@sourceware.org> On Behalf Of Metzger, Markus T via Gdb-patches
Sent: Tuesday, December 8, 2020 9:48 AM
To: Zaric, Zoran (Zare) <Zoran.Zaric@amd.com>
Cc: gdb-patches@sourceware.org
Subject: RE: [PATCH 00/30] Allow location description on the DWARF stack

[CAUTION: External Email]

Hello Zoran,

> The proposed implementation in this patch series is completely 
> backward compatible with the DWARF 5 standard.

Wouldn't this change the semantics of existing operators?

DW_OP_piece, for example, does not push the value onto the DWARF stack today; also, it terminates a DWARF sub-expression, which is no longer the case with your proposal.

I like this extension (referring to location descriptions and allowing pieces on the stack; I have not read the full proposal) as it addresses a problem we're also facing.  But I'm not sure about the proper ordering of things.

Wouldn't this make GDB non-compliant?  Is there some opt-in so GDB can recognize producers that are aware of this?

Do you have these patches on a user branch?

regards,
Markus.

> -----Original Message-----
> From: Gdb-patches <gdb-patches-bounces@sourceware.org> On Behalf Of 
> Zoran Zaric via Gdb-patches
> Sent: Montag, 7. Dezember 2020 20:00
> To: gdb-patches@sourceware.org
> Cc: Zoran Zaric <Zoran.Zaric@amd.com>
> Subject: [PATCH 00/30] Allow location description on the DWARF stack
>
> Based on gdb master: 13f11b0b61ca2620611b08eeaece0ce62c862f4b
>
> The idea of this patch series is to cleanup the design of the DWARF 
> expression evaluator (dwarf_expr_context class) and allow a future 
> extensions of that evaluator needed for the upcoming vendor specific 
> DWARF extensions.
>
> Main motivation behind this series of patches is AMD’s effort to 
> improve DWARF support for heavily optimized code, but more 
> specifically, optimized code for SIMD and SIMT architectures.
>
> These patches are part of the AMD’s DWARF standard extensions that can 
> be found at:
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fllvm
> .org%2Fdocs%2FAMDGPUDwarfExtensionsForHeterogeneousDebugging.html&amp;
> data=04%7C01%7Ctony.tye%40amd.com%7Cdc391fc1b8c64df8009408d89b8855c1%7
> C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637430357142209676%7CUnkno
> wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> CJXVCI6Mn0%3D%7C1000&amp;sdata=UmXqMslVerr06k4NgA83e6vJf63dfxl3VFzipev
> UA%2BQ%3D&amp;reserved=0
>
> While trying to support optimized code for these architectures, we 
> found couple of restriction imposed by the version 5 of the standard:
>
> - CFI describes restoring callee saved registers that are spilled.
>   Currently CFI only allows a location description that is a register,
>   memory address, or implicit location description. AMDGPU optimized
>   code may spill scalar registers into portions of vector registers.
>   This requires extending CFI to allow any location description.
>
> - Optimized code may need to describe a variable that resides in pieces
>   that are in different kinds of storage which may include parts that
>   are in different kinds of memory address spaces. DWARF has the
>   concept of segment addresses. However, the segment cannot be
>   specified within a DWARF expression, which is only able to specify
>   the offset portion of a segment address. The segment index is only
>   provided by the entity that specifies the DWARF expression.
>   Therefore, the segment index is a property that can only be put on
>   complete objects, such as a variable. Another problem is with DWARF
>   DW_OP_xderef* operations, which allow a value to be converted into
>   an address of a specified address space which is then read. But it
>   provides no way to create a memory location description for an
>   address in the non-default address space.
>
> - DW_OP_breg* treats the register as containing an address in the
>   default address space. It is required to be able to specify the
>   address space of the register value.
>
> - There is really limited support for bit offsets of location
>   description. This issue was already found by others who worked
>   on packed array support for ADA language. The array could be packed
>   in such a way that their start is not byte alligned. There is no way
>   to describe this in DWARF, so gdb implementation in required for the
>   packed array to be copied to the byte alligned addresses and then
>   passed that buffer in the expression evaluator. This approach is
>   restrictive for more complex scenarios.
>
> All these restriction are caused by the fact that DWARF stack can only 
> hold values which size is not bigger then the size of the DWARF 
> generic type. This means that intermediate result of any DWARF 
> operation needs to fit in such a small representation.
>
> DWARF Version 5 does not allow location descriptions to be entries on 
> the DWARF stack. They can only be the final result of the evaluation 
> of a DWARF expression. However, by allowing a location description to 
> be a first-class entry on the DWARF stack it becomes possible to 
> compose expressions containing both values and location descriptions naturally.
> It allows objects to be located in any kind of memory address space, 
> in registers, be implicit values, be undefined, or a composite of any 
> of these.
>
> This approach unifies the location description operations with general 
> operations. This in turn allows addition of new DWARF operations which 
> can push a memory location description with any offset and in any kind 
> of memory address space.
>
> The number of registers and the cost of memory operations is much 
> higher for some architectures than a typical CPU. The compiler 
> attempts to optimize whole variables and arrays into registers. 
> Currently DWARF only allows DW_OP_push_object_address and related 
> operations to work with a global memory location. With this change, 
> push object address mechanism can be modified to work with any 
> location description. This would also removed the need for the passed 
> in buffer mechanism that ADA and GO languages currently use.
>
> The proposed implementation in this patch series is completely 
> backward compatible with the DWARF 5 standard.
>
> Although the patch series is designed as a whole, the first fifteen 
> patches could be viewed as a standalone subset that introduces a 
> welcome cleanup of the DWARF expression evaluator module.
>
> The patch series is based on a patch that is already in the review 
> process, so it had to be included in this series as well. The review 
> for that patch can be found here:
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
> ceware.org%2Fpipermail%2Fgdb-patches%2F2020-November%2F173638.html&amp
> ;data=04%7C01%7Ctony.tye%40amd.com%7Cdc391fc1b8c64df8009408d89b8855c1%
> 7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637430357142209676%7CUnkn
> own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> LCJXVCI6Mn0%3D%7C1000&amp;sdata=m6SzVi7zrym1UCNJRwOuDYAfNr0xww9beANmvr
> mb%2B0s%3D&amp;reserved=0
>
> But I don't mind it being discussed as a part of this patch series.
>
> Zoran Zaric (30):
>   Replace the symbol needs evaluator with a parser
>   Move frame context info to dwarf_expr_context
>   Remove get_frame_cfa from dwarf_expr_context
>   Move compilation unit info to dwarf_expr_context
>   Move dwarf_call to dwarf_expr_context
>   Move get_object_address to dwarf_expr_context
>   Move read_mem to dwarf_expr_context
>   Move push_dwarf_reg_entry_value to expr.c
>   Inline get_reg_value method of dwarf_expr_context
>   Remove empty frame and full evaluators
>   Merge evaluate_for_locexpr_baton evaluator
>   Move piece_closure and its support to expr.c
>   Make DWARF evaluator return a single struct value
>   Simplify dwarf_expr_context class interface
>   Add as_lval argument to expression evaluator
>   Add new register access interface to expr.c
>   Add new memory access interface to expr.c
>   Add new classes that model DWARF stack element
>   Add new location description access interface
>   Add dwarf_entry factory class to expr.c
>   Change DWARF stack to use new dwarf_entry classes
>   Remove dwarf_expr_context from expr.h interface
>   Rename and update the piece_closure structure
>   Move read_addr_from_reg function to frame.c
>   Add frame info check to DW_OP_reg operations
>   Remove DWARF expression composition check
>   Add support for any location description in CFI
>   Add DWARF operations for byte and bit offset
>   Add support for DW_OP_LLVM_undefined operation
>   Add support for nested composite locations
>
>  gdb/ada-lang.c                                |    2 +-
>  gdb/compile/compile-loc2c.c                   |   11 +
>  gdb/dwarf2/expr.c                             | 3406 +++++++++++++++--
>  gdb/dwarf2/expr.h                             |  275 +-
>  gdb/dwarf2/frame.c                            |  163 +-
>  gdb/dwarf2/loc.c                              | 1885 +++------
>  gdb/dwarf2/loc.h                              |   34 +-
>  gdb/f-lang.c                                  |    2 +-
>  gdb/findvar.c                                 |    4 +-
>  gdb/frame.c                                   |   36 +-
>  gdb/testsuite/gdb.dwarf2/dw2-llvm-offset.exp  |  328 ++
>  .../gdb.dwarf2/dw2-llvm-piece-end.exp         |  191 +
>  .../gdb.dwarf2/dw2-llvm-undefined.exp         |  144 +
>  gdb/testsuite/gdb.dwarf2/symbol_needs_eval.c  |   25 +
>  .../gdb.dwarf2/symbol_needs_eval_fail.exp     |  108 +
>  .../gdb.dwarf2/symbol_needs_eval_timeout.exp  |  127 +
>  .../amd64-py-framefilter-invalidarg.S         |    1 -
>  .../gdb.python/py-framefilter-invalidarg.exp  |    2 +-
>  gdb/testsuite/lib/dwarf.exp                   |    8 +
>  gdb/valops.c                                  |  123 +-
>  gdb/value.c                                   |   68 +-
>  gdb/value.h                                   |    6 +-
>  include/dwarf2.def                            |    6 +
>  23 files changed, 4841 insertions(+), 2114 deletions(-)  create mode 
> 100644 gdb/testsuite/gdb.dwarf2/dw2-llvm-offset.exp
>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-llvm-piece-end.exp
>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-llvm-undefined.exp
>  create mode 100644 gdb/testsuite/gdb.dwarf2/symbol_needs_eval.c
>  create mode 100644 
> gdb/testsuite/gdb.dwarf2/symbol_needs_eval_fail.exp
>  create mode 100644 
> gdb/testsuite/gdb.dwarf2/symbol_needs_eval_timeout.exp
>
> --
> 2.17.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.intel.de%2F&amp;data=04%7C01%7Ctony.tye%40amd.com%7Cdc391fc1b8c64df8009408d89b8855c1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637430357142209676%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=uO9stvNjdJ61MIzPiJydQdL0wzxq5UtfmMeuz1QSQAs%3D&amp;reserved=0
Managing Directors: Christin Eisenschmid, Gary Kershaw Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928