* [PATCH] DWARF reader: use size_t for DWARF expression length @ 2022-01-17 14:51 Giuliano Procida 2022-01-18 9:52 ` Mark Wielaard 2022-01-18 12:03 ` Dodji Seketeli 0 siblings, 2 replies; 4+ messages in thread From: Giuliano Procida @ 2022-01-17 14:51 UTC (permalink / raw) To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich, mark A recent change broke 32-bit builds due to an implicit assumption that size_t == uint64_t. This appears in the elfutils dwarf_getlocation* functions' types. This commit updates callers and other functions to use size_t consistently for such expression lengths and indexes. * src/abg-dwarf-reader.cc (die_location_expr): Change expr_len argument type to size_t*. (op_manipulates_stack): Change expr_len and index argument types to size_t; change next_index argument type to size_t&. (eval_last_constant_dwarf_sub_expr): Change expr_len argument and local variables index and next_index types to size_t. (die_member_offset): Change local variable expr_len type to size_t. (die_location_address): Likewise. (die_virtual_function_index): Likewise. Fixes: 16207c4af7bc ("Bug 28191 - Interpret DWARF 5 addrx locations") Signed-off-by: Giuliano Procida <gprocida@google.com> --- src/abg-dwarf-reader.cc | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc index f125196d..7ee8c04a 100644 --- a/src/abg-dwarf-reader.cc +++ b/src/abg-dwarf-reader.cc @@ -7686,7 +7686,7 @@ static bool die_location_expr(const Dwarf_Die* die, unsigned attr_name, Dwarf_Op** expr, - uint64_t* expr_len) + size_t* expr_len) { if (!die) return false; @@ -8022,9 +8022,9 @@ op_pushes_non_constant_value(Dwarf_Op* ops, /// DEVM stack, false otherwise. static bool op_manipulates_stack(Dwarf_Op* expr, - uint64_t expr_len, - uint64_t index, - uint64_t& next_index, + size_t expr_len, + size_t index, + size_t& next_index, dwarf_expr_eval_context& ctxt) { Dwarf_Op& op = expr[index]; @@ -8398,7 +8398,7 @@ eval_quickly(Dwarf_Op* expr, /// to evaluate, false otherwise. static bool eval_last_constant_dwarf_sub_expr(Dwarf_Op* expr, - uint64_t expr_len, + size_t expr_len, int64_t& value, bool& is_tls_address, dwarf_expr_eval_context &eval_ctxt) @@ -8407,7 +8407,7 @@ eval_last_constant_dwarf_sub_expr(Dwarf_Op* expr, // expression contained in the DWARF expression 'expr'. eval_ctxt.reset(); - uint64_t index = 0, next_index = 0; + size_t index = 0, next_index = 0; do { if (op_is_arith_logic(expr, expr_len, index, @@ -8452,7 +8452,7 @@ eval_last_constant_dwarf_sub_expr(Dwarf_Op* expr, /// to evaluate, false otherwise. static bool eval_last_constant_dwarf_sub_expr(Dwarf_Op* expr, - uint64_t expr_len, + size_t expr_len, int64_t& value, bool& is_tls_address) { @@ -8775,7 +8775,7 @@ die_member_offset(const read_context& ctxt, int64_t& offset) { Dwarf_Op* expr = NULL; - uint64_t expr_len = 0; + size_t expr_len = 0; uint64_t bit_offset = 0; // First let's see if the DW_AT_data_bit_offset attribute is @@ -8852,7 +8852,7 @@ die_location_address(Dwarf_Die* die, bool& is_tls_address) { Dwarf_Op* expr = NULL; - uint64_t expr_len = 0; + size_t expr_len = 0; is_tls_address = false; @@ -8898,7 +8898,7 @@ die_virtual_function_index(Dwarf_Die* die, return false; Dwarf_Op* expr = NULL; - uint64_t expr_len = 0; + size_t expr_len = 0; if (!die_location_expr(die, DW_AT_vtable_elem_location, &expr, &expr_len)) return false; -- 2.34.1.703.g22d0c6ccf7-goog ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] DWARF reader: use size_t for DWARF expression length 2022-01-17 14:51 [PATCH] DWARF reader: use size_t for DWARF expression length Giuliano Procida @ 2022-01-18 9:52 ` Mark Wielaard 2022-01-18 9:57 ` Giuliano Procida 2022-01-18 12:03 ` Dodji Seketeli 1 sibling, 1 reply; 4+ messages in thread From: Mark Wielaard @ 2022-01-18 9:52 UTC (permalink / raw) To: Giuliano Procida; +Cc: libabigail, dodji, kernel-team, maennich [-- Attachment #1: Type: text/plain, Size: 1089 bytes --] Hi Giuliano, On Mon, Jan 17, 2022 at 02:51:15PM +0000, Giuliano Procida wrote: > A recent change broke 32-bit builds due to an implicit assumption that > size_t == uint64_t. This appears in the elfutils dwarf_getlocation* > functions' types. > > This commit updates callers and other functions to use size_t > consistently for such expression lengths and indexes. > > * src/abg-dwarf-reader.cc (die_location_expr): Change expr_len > argument type to size_t*. > (op_manipulates_stack): Change expr_len and index argument > types to size_t; change next_index argument type to size_t&. > (eval_last_constant_dwarf_sub_expr): Change expr_len argument > and local variables index and next_index types to size_t. > (die_member_offset): Change local variable expr_len type to > size_t. > (die_location_address): Likewise. > (die_virtual_function_index): Likewise. I also needed to change the len and index arguments of eval_last_constant_dwarf_sub_expr, op_pushes_constant_value, op_pushes_non_constant_value, op_is_arith_logic and op_is_control_flow. See attached. Cheers, Mark [-- Attachment #2: p --] [-- Type: text/plain, Size: 4135 bytes --] diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc index f125196d..d8545b4c 100644 --- a/src/abg-dwarf-reader.cc +++ b/src/abg-dwarf-reader.cc @@ -363,7 +363,7 @@ die_is_at_class_scope(const read_context& ctxt, Dwarf_Die& class_scope_die); static bool eval_last_constant_dwarf_sub_expr(Dwarf_Op* expr, - uint64_t expr_len, + size_t expr_len, int64_t& value, bool& is_tls_address); @@ -7686,7 +7686,7 @@ static bool die_location_expr(const Dwarf_Die* die, unsigned attr_name, Dwarf_Op** expr, - uint64_t* expr_len) + size_t* expr_len) { if (!die) return false; @@ -7732,9 +7732,9 @@ die_location_expr(const Dwarf_Die* die, /// value onto the DEVM stack, false otherwise. static bool op_pushes_constant_value(Dwarf_Op* ops, - uint64_t ops_len, - uint64_t index, - uint64_t& next_index, + size_t ops_len, + size_t index, + size_t& next_index, dwarf_expr_eval_context& ctxt) { ABG_ASSERT(index < ops_len); @@ -7896,9 +7896,9 @@ op_pushes_constant_value(Dwarf_Op* ops, /// non-constant value onto the DEVM stack, false otherwise. static bool op_pushes_non_constant_value(Dwarf_Op* ops, - uint64_t ops_len, - uint64_t index, - uint64_t& next_index, + size_t ops_len, + size_t index, + size_t& next_index, dwarf_expr_eval_context& ctxt) { ABG_ASSERT(index < ops_len); @@ -8022,9 +8022,9 @@ op_pushes_non_constant_value(Dwarf_Op* ops, /// DEVM stack, false otherwise. static bool op_manipulates_stack(Dwarf_Op* expr, - uint64_t expr_len, - uint64_t index, - uint64_t& next_index, + size_t expr_len, + size_t index, + size_t& next_index, dwarf_expr_eval_context& ctxt) { Dwarf_Op& op = expr[index]; @@ -8146,9 +8146,9 @@ op_manipulates_stack(Dwarf_Op* expr, /// arithmetic or logic operation. static bool op_is_arith_logic(Dwarf_Op* expr, - uint64_t expr_len, - uint64_t index, - uint64_t& next_index, + size_t expr_len, + size_t index, + size_t& next_index, dwarf_expr_eval_context& ctxt) { ABG_ASSERT(index < expr_len); @@ -8279,9 +8279,9 @@ op_is_arith_logic(Dwarf_Op* expr, /// control flow operation, false otherwise. static bool op_is_control_flow(Dwarf_Op* expr, - uint64_t expr_len, - uint64_t index, - uint64_t& next_index, + size_t expr_len, + size_t index, + size_t& next_index, dwarf_expr_eval_context& ctxt) { ABG_ASSERT(index < expr_len); @@ -8398,7 +8398,7 @@ eval_quickly(Dwarf_Op* expr, /// to evaluate, false otherwise. static bool eval_last_constant_dwarf_sub_expr(Dwarf_Op* expr, - uint64_t expr_len, + size_t expr_len, int64_t& value, bool& is_tls_address, dwarf_expr_eval_context &eval_ctxt) @@ -8407,7 +8407,7 @@ eval_last_constant_dwarf_sub_expr(Dwarf_Op* expr, // expression contained in the DWARF expression 'expr'. eval_ctxt.reset(); - uint64_t index = 0, next_index = 0; + size_t index = 0, next_index = 0; do { if (op_is_arith_logic(expr, expr_len, index, @@ -8452,7 +8452,7 @@ eval_last_constant_dwarf_sub_expr(Dwarf_Op* expr, /// to evaluate, false otherwise. static bool eval_last_constant_dwarf_sub_expr(Dwarf_Op* expr, - uint64_t expr_len, + size_t expr_len, int64_t& value, bool& is_tls_address) { @@ -8775,7 +8775,7 @@ die_member_offset(const read_context& ctxt, int64_t& offset) { Dwarf_Op* expr = NULL; - uint64_t expr_len = 0; + size_t expr_len = 0; uint64_t bit_offset = 0; // First let's see if the DW_AT_data_bit_offset attribute is @@ -8852,7 +8852,7 @@ die_location_address(Dwarf_Die* die, bool& is_tls_address) { Dwarf_Op* expr = NULL; - uint64_t expr_len = 0; + size_t expr_len = 0; is_tls_address = false; @@ -8898,7 +8898,7 @@ die_virtual_function_index(Dwarf_Die* die, return false; Dwarf_Op* expr = NULL; - uint64_t expr_len = 0; + size_t expr_len = 0; if (!die_location_expr(die, DW_AT_vtable_elem_location, &expr, &expr_len)) return false; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] DWARF reader: use size_t for DWARF expression length 2022-01-18 9:52 ` Mark Wielaard @ 2022-01-18 9:57 ` Giuliano Procida 0 siblings, 0 replies; 4+ messages in thread From: Giuliano Procida @ 2022-01-18 9:57 UTC (permalink / raw) To: Mark Wielaard; +Cc: libabigail, dodji, kernel-team, maennich The latest merged commit doesn't have the extra fixes you mentioned. I'm happy to replicate your work, but I risk leaving something out again. Let me know, Giuliano. On Tue, 18 Jan 2022 at 09:52, Mark Wielaard <mark@klomp.org> wrote: > > Hi Giuliano, > > On Mon, Jan 17, 2022 at 02:51:15PM +0000, Giuliano Procida wrote: > > A recent change broke 32-bit builds due to an implicit assumption that > > size_t == uint64_t. This appears in the elfutils dwarf_getlocation* > > functions' types. > > > > This commit updates callers and other functions to use size_t > > consistently for such expression lengths and indexes. > > > > * src/abg-dwarf-reader.cc (die_location_expr): Change expr_len > > argument type to size_t*. > > (op_manipulates_stack): Change expr_len and index argument > > types to size_t; change next_index argument type to size_t&. > > (eval_last_constant_dwarf_sub_expr): Change expr_len argument > > and local variables index and next_index types to size_t. > > (die_member_offset): Change local variable expr_len type to > > size_t. > > (die_location_address): Likewise. > > (die_virtual_function_index): Likewise. > > I also needed to change the len and index arguments of > eval_last_constant_dwarf_sub_expr, op_pushes_constant_value, > op_pushes_non_constant_value, op_is_arith_logic and > op_is_control_flow. > > See attached. > > Cheers, > > Mark ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] DWARF reader: use size_t for DWARF expression length 2022-01-17 14:51 [PATCH] DWARF reader: use size_t for DWARF expression length Giuliano Procida 2022-01-18 9:52 ` Mark Wielaard @ 2022-01-18 12:03 ` Dodji Seketeli 1 sibling, 0 replies; 4+ messages in thread From: Dodji Seketeli @ 2022-01-18 12:03 UTC (permalink / raw) To: Giuliano Procida; +Cc: libabigail, kernel-team, maennich, mark Giuliano Procida <gprocida@google.com> a écrit: > A recent change broke 32-bit builds due to an implicit assumption that > size_t == uint64_t. This appears in the elfutils dwarf_getlocation* > functions' types. > > This commit updates callers and other functions to use size_t > consistently for such expression lengths and indexes. > > * src/abg-dwarf-reader.cc (die_location_expr): Change expr_len > argument type to size_t*. > (op_manipulates_stack): Change expr_len and index argument > types to size_t; change next_index argument type to size_t&. > (eval_last_constant_dwarf_sub_expr): Change expr_len argument > and local variables index and next_index types to size_t. > (die_member_offset): Change local variable expr_len type to > size_t. > (die_location_address): Likewise. > (die_virtual_function_index): Likewise. > > Fixes: 16207c4af7bc ("Bug 28191 - Interpret DWARF 5 addrx locations") > Signed-off-by: Giuliano Procida <gprocida@google.com> Applied to master, thanks! [...] Cheers, -- Dodji ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-18 12:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-17 14:51 [PATCH] DWARF reader: use size_t for DWARF expression length Giuliano Procida 2022-01-18 9:52 ` Mark Wielaard 2022-01-18 9:57 ` Giuliano Procida 2022-01-18 12:03 ` Dodji Seketeli
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).