* [PATCH] Allow DW_OP_GNU_uninit in dwarf_expr_require_composition
@ 2016-04-27 17:38 Andreas Arnez
2016-10-04 17:33 ` Pedro Alves
2016-10-09 17:36 ` Tom Tromey
0 siblings, 2 replies; 6+ messages in thread
From: Andreas Arnez @ 2016-04-27 17:38 UTC (permalink / raw)
To: gdb-patches
In DWARF expression handling, some operators are required to be either
at the end of an expression or followed by a composition operator. So
far only the operators DW_OP_reg0-31 were allowed to be followed by
DW_OP_GNU_uninit instead, and particularly DW_OP_regx was not, which is
obviously inconsistent.
This patch allows DW_OP_GNU_uninit after all operators requiring a
composition, to simplify the code and make it more consistent. This
policy may be more permissive than necessary, but in the worst case just
leads to a DWARF location description resulting in an uninitialized
value instead of an error message.
gdb/ChangeLog:
* dwarf2expr.c (dwarf_expr_require_composition): Allow
DW_OP_GNU_uninit.
(execute_stack_op): Use dwarf_expr_require_composition instead of
copying its logic.
---
gdb/dwarf2expr.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/gdb/dwarf2expr.c b/gdb/dwarf2expr.c
index 7bf3c78..bf2107a 100644
--- a/gdb/dwarf2expr.c
+++ b/gdb/dwarf2expr.c
@@ -401,16 +401,15 @@ safe_skip_leb128 (const gdb_byte *buf, const gdb_byte *buf_end)
\f
/* Check that the current operator is either at the end of an
- expression, or that it is followed by a composition operator. */
+ expression, or that it is followed by a composition operator or by
+ DW_OP_GNU_uninit (which should terminate the expression). */
void
dwarf_expr_require_composition (const gdb_byte *op_ptr, const gdb_byte *op_end,
const char *op_name)
{
- /* It seems like DW_OP_GNU_uninit should be handled here. However,
- it doesn't seem to make sense for DW_OP_*_value, and it was not
- checked at the other place that this function is called. */
- if (op_ptr != op_end && *op_ptr != DW_OP_piece && *op_ptr != DW_OP_bit_piece)
+ if (op_ptr != op_end && *op_ptr != DW_OP_piece && *op_ptr != DW_OP_bit_piece
+ && *op_ptr != DW_OP_GNU_uninit)
error (_("DWARF-2 expression error: `%s' operations must be "
"used either alone or in conjunction with DW_OP_piece "
"or DW_OP_bit_piece."),
@@ -818,13 +817,7 @@ execute_stack_op (struct dwarf_expr_context *ctx,
case DW_OP_reg29:
case DW_OP_reg30:
case DW_OP_reg31:
- if (op_ptr != op_end
- && *op_ptr != DW_OP_piece
- && *op_ptr != DW_OP_bit_piece
- && *op_ptr != DW_OP_GNU_uninit)
- error (_("DWARF-2 expression error: DW_OP_reg operations must be "
- "used either alone or in conjunction with DW_OP_piece "
- "or DW_OP_bit_piece."));
+ dwarf_expr_require_composition (op_ptr, op_end, "DW_OP_reg");
result = op - DW_OP_reg0;
result_val = value_from_ulongest (address_type, result);
--
2.3.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Allow DW_OP_GNU_uninit in dwarf_expr_require_composition
2016-04-27 17:38 [PATCH] Allow DW_OP_GNU_uninit in dwarf_expr_require_composition Andreas Arnez
@ 2016-10-04 17:33 ` Pedro Alves
2016-10-05 10:42 ` Andreas Arnez
2016-10-09 17:36 ` Tom Tromey
1 sibling, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2016-10-04 17:33 UTC (permalink / raw)
To: Andreas Arnez, gdb-patches
On 04/27/2016 06:38 PM, Andreas Arnez wrote:
> In DWARF expression handling, some operators are required to be either
> at the end of an expression or followed by a composition operator. So
> far only the operators DW_OP_reg0-31 were allowed to be followed by
> DW_OP_GNU_uninit instead, and particularly DW_OP_regx was not, which is
> obviously inconsistent.
>
> This patch allows DW_OP_GNU_uninit after all operators requiring a
> composition, to simplify the code and make it more consistent. This
> policy may be more permissive than necessary, but in the worst case just
> leads to a DWARF location description resulting in an uninitialized
> value instead of an error message.
Fine with me. DW_OP_GNU_uninit is underspecified, anyway.
>
> gdb/ChangeLog:
>
> * dwarf2expr.c (dwarf_expr_require_composition): Allow
> DW_OP_GNU_uninit.
> (execute_stack_op): Use dwarf_expr_require_composition instead of
> copying its logic.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Allow DW_OP_GNU_uninit in dwarf_expr_require_composition
2016-10-04 17:33 ` Pedro Alves
@ 2016-10-05 10:42 ` Andreas Arnez
0 siblings, 0 replies; 6+ messages in thread
From: Andreas Arnez @ 2016-10-05 10:42 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Tue, Oct 04 2016, Pedro Alves wrote:
>> gdb/ChangeLog:
>>
>> * dwarf2expr.c (dwarf_expr_require_composition): Allow
>> DW_OP_GNU_uninit.
>> (execute_stack_op): Use dwarf_expr_require_composition instead of
>> copying its logic.
>
> OK.
Thanks, pushed.
--
Andreas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Allow DW_OP_GNU_uninit in dwarf_expr_require_composition
2016-04-27 17:38 [PATCH] Allow DW_OP_GNU_uninit in dwarf_expr_require_composition Andreas Arnez
2016-10-04 17:33 ` Pedro Alves
@ 2016-10-09 17:36 ` Tom Tromey
2016-10-10 12:24 ` Andreas Arnez
1 sibling, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2016-10-09 17:36 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
>>>>> "Andreas" == Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
Andreas> This patch allows DW_OP_GNU_uninit after all operators
Andreas> requiring a composition, to simplify the code and make it more
Andreas> consistent. This policy may be more permissive than necessary,
Andreas> but in the worst case just leads to a DWARF location
Andreas> description resulting in an uninitialized value instead of an
Andreas> error message.
I think it would be best to allow DW_OP_GNU_uninit to terminate any
piece, rather than require it to be at the end of the expression. This
seems compatible and clearly more consistent with other DWARF
operations.
That is, assuming DW_OP_GNU_uninit is useful at all.
Another option would be to deprecate it.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Allow DW_OP_GNU_uninit in dwarf_expr_require_composition
2016-10-09 17:36 ` Tom Tromey
@ 2016-10-10 12:24 ` Andreas Arnez
2016-10-10 22:41 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Arnez @ 2016-10-10 12:24 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Sun, Oct 09 2016, Tom Tromey wrote:
>>>>>> "Andreas" == Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
>
> Andreas> This patch allows DW_OP_GNU_uninit after all operators
> Andreas> requiring a composition, to simplify the code and make it more
> Andreas> consistent. This policy may be more permissive than necessary,
> Andreas> but in the worst case just leads to a DWARF location
> Andreas> description resulting in an uninitialized value instead of an
> Andreas> error message.
>
> I think it would be best to allow DW_OP_GNU_uninit to terminate any
> piece, rather than require it to be at the end of the expression. This
> seems compatible and clearly more consistent with other DWARF
> operations.
You mean to allow DW_OP_GNU_uninit to terminate any simple location
description? Right, this would allow marking individual structure
members as uninitialized, for instance.
> That is, assuming DW_OP_GNU_uninit is useful at all.
> Another option would be to deprecate it.
Right, I wonder about its usefulness as well. For a variable with fixed
location it may cover a small window where the compiler can be certain
that the variable is uninitialized. I guess this *might* be useful
sometimes?
Is there even a DWARF issue for this? Or a formal specification?
--
Andreas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Allow DW_OP_GNU_uninit in dwarf_expr_require_composition
2016-10-10 12:24 ` Andreas Arnez
@ 2016-10-10 22:41 ` Tom Tromey
0 siblings, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2016-10-10 22:41 UTC (permalink / raw)
To: Andreas Arnez; +Cc: Tom Tromey, gdb-patches
>>>>> "Andreas" == Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
Andreas> You mean to allow DW_OP_GNU_uninit to terminate any simple
Andreas> location description?
Yes.
>> That is, assuming DW_OP_GNU_uninit is useful at all.
>> Another option would be to deprecate it.
Andreas> Right, I wonder about its usefulness as well. For a variable
Andreas> with fixed location it may cover a small window where the
Andreas> compiler can be certain that the variable is uninitialized. I
Andreas> guess this *might* be useful sometimes?
Andreas> Is there even a DWARF issue for this? Or a formal
Andreas> specification?
I don't think so. Last time I looked into this all I was able to find
were the patch submissions to gcc and gdb. IIRC they weren't all that
informative though.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-10 22:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-27 17:38 [PATCH] Allow DW_OP_GNU_uninit in dwarf_expr_require_composition Andreas Arnez
2016-10-04 17:33 ` Pedro Alves
2016-10-05 10:42 ` Andreas Arnez
2016-10-09 17:36 ` Tom Tromey
2016-10-10 12:24 ` Andreas Arnez
2016-10-10 22:41 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).