* [PATCH] readelf: Remove show_op_index variable
@ 2021-02-02 10:23 tbaeder
2021-02-05 15:46 ` Mark Wielaard
0 siblings, 1 reply; 2+ messages in thread
From: tbaeder @ 2021-02-02 10:23 UTC (permalink / raw)
To: elfutils-devel
From: Timm Bäder <tbaeder@redhat.com>
advance_pc() uses show_op_index to save whether the current op_index is
> 0 OR the new op_index is > 0.
The new op index is calculated via
new_op_index = (op_index + op_advance) % max_ops_per_instr;
since all of the variables involved are unsigned,
new_op_index >= op_index is always true.
So...
if op_index > 0, then new_op_index > 0
if op_index == 0, then new_op_index >= 0
and if the new_op_index is > 0, then the old one was as well.
In any case, we only need to check the new_op_index, since show_op_index
used to OR the two comparisons.
In other words:
op_index > 0 | new_op_index > 0 || show_op_index
------------------------------------------------
true true true
false true true
true false true xx
false false false
... but since the third line (marked with xx) is not possible,
the table becomes:
op_index > 0 | new_op_index > 0 || show_op_index
------------------------------------------------
true true true
false true true
false false false
... and show_op_index is equal to (new_op_index > 0).
So, remove the show_op_index variable and simply replace it by comparing
the new op_index > 0.
---
src/ChangeLog | 5 +++++
src/readelf.c | 9 +++------
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 1f53ca7b..9e30df31 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2021-02-02 Timm Bäder <tbaeder@redhat.com>
+
+ * readelf.c (print_debug_line_section): Remove unnecessary
+ show_op_index variable, replace with (op_index > 0).
+
2021-01-08 Timm Bäder <tbaeder@redhat.com>
* readelf.c (print_cfa_program): Lift regname function to...
diff --git a/src/readelf.c b/src/readelf.c
index 9943c211..11692bb5 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -8752,14 +8752,11 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr,
/* Apply the "operation advance" from a special opcode
or DW_LNS_advance_pc (as per DWARF4 6.2.5.1). */
unsigned int op_addr_advance;
- bool show_op_index;
inline void advance_pc (unsigned int op_advance)
{
op_addr_advance = minimum_instr_len * ((op_index + op_advance)
/ max_ops_per_instr);
address += op_addr_advance;
- show_op_index = (op_index > 0 ||
- (op_index + op_advance) % max_ops_per_instr > 0);
op_index = (op_index + op_advance) % max_ops_per_instr;
}
@@ -8803,7 +8800,7 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr,
printf (_(" special opcode %u: address+%u = "),
opcode, op_addr_advance);
print_dwarf_addr (dwflmod, 0, address, address);
- if (show_op_index)
+ if (op_index > 0)
printf (_(", op_index = %u, line%+d = %zu\n"),
op_index, line_increment, line);
else
@@ -8921,7 +8918,7 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr,
printf (_(" advance address by %u to "),
op_addr_advance);
print_dwarf_addr (dwflmod, 0, address, address);
- if (show_op_index)
+ if (op_index > 0)
printf (_(", op_index to %u"), op_index);
printf ("\n");
}
@@ -8982,7 +8979,7 @@ print_debug_line_section (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr,
printf (_(" advance address by constant %u to "),
op_addr_advance);
print_dwarf_addr (dwflmod, 0, address, address);
- if (show_op_index)
+ if (op_index > 0)
printf (_(", op_index to %u"), op_index);
printf ("\n");
}
--
2.26.2
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] readelf: Remove show_op_index variable
2021-02-02 10:23 [PATCH] readelf: Remove show_op_index variable tbaeder
@ 2021-02-05 15:46 ` Mark Wielaard
0 siblings, 0 replies; 2+ messages in thread
From: Mark Wielaard @ 2021-02-05 15:46 UTC (permalink / raw)
To: tbaeder, elfutils-devel
Hi Timm,
On Tue, 2021-02-02 at 11:23 +0100, Timm Bäder via Elfutils-devel wrote:
> advance_pc() uses show_op_index to save whether the current op_index
> is
> > 0 OR the new op_index is > 0.
>
> The new op index is calculated via
>
> new_op_index = (op_index + op_advance) % max_ops_per_instr;
>
> since all of the variables involved are unsigned,
> new_op_index >= op_index is always true.
>
> So...
>
> if op_index > 0, then new_op_index > 0
> if op_index == 0, then new_op_index >= 0
>
> and if the new_op_index is > 0, then the old one was as well.
>
> In any case, we only need to check the new_op_index, since show_op_index
> used to OR the two comparisons.
>
> In other words:
>
> op_index > 0 | new_op_index > 0 || show_op_index
> ------------------------------------------------
> true true true
> false true true
> true false true xx
> false false false
>
> ... but since the third line (marked with xx) is not possible,
> the table becomes:
>
> op_index > 0 | new_op_index > 0 || show_op_index
> ------------------------------------------------
> true true true
> false true true
> false false false
>
> ... and show_op_index is equal to (new_op_index > 0).
> So, remove the show_op_index variable and simply replace it by comparing
> the new op_index > 0.
Very nice simplification. Thanks for the extensive commit message.
In most cases max_ops_per_instr is actually 1. In which case op_index
will always be zero.
Pushed to trunk.
Thanks,
Mark
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-02-05 15:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02 10:23 [PATCH] readelf: Remove show_op_index variable tbaeder
2021-02-05 15:46 ` Mark Wielaard
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).