[v5,1/2,gdb/symtab] Fix an out of bounds array access in find_epilogue_using_linetable
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
An out of bounds array access in find_epilogue_using_linetable causes random
test failures like these:
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $fba_value == $fn_fba
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: check frame-id matches
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: bt 2
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: up
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $sp_value == $::main_sp
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $fba_value == $::main_fba
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: [string equal $fid $::main_fid]
Here the read happens below the first element of the line
table, and the test failure depends on the value that is
read from there.
It also happens that std::lower_bound returns a pointer exactly at the upper
bound of the line table, also here the read value is undefined, that happens
in this test:
FAIL: gdb.dwarf2/dw2-epilogue-begin.exp: confirm watchpoint doesn't trigger
Fixes: 528b729be1a2 ("gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table")
Co-Authored-By: Tom de Vries <tdevries@suse.de>
PR symtab/31268
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31268
---
gdb/symtab.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 84 insertions(+), 10 deletions(-)
base-commit: 9132c8152b899a1683bc886f8ba76bedadb48aa1
Comments
On 4/9/24 11:27, Tom de Vries wrote:
> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
>
> An out of bounds array access in find_epilogue_using_linetable causes random
> test failures like these:
>
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $fba_value == $fn_fba
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: check frame-id matches
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: bt 2
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: up
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $sp_value == $::main_sp
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $fba_value == $::main_fba
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: [string equal $fid $::main_fid]
>
> Here the read happens below the first element of the line
> table, and the test failure depends on the value that is
> read from there.
>
> It also happens that std::lower_bound returns a pointer exactly at the upper
> bound of the line table, also here the read value is undefined, that happens
> in this test:
>
> FAIL: gdb.dwarf2/dw2-epilogue-begin.exp: confirm watchpoint doesn't trigger
>
> Fixes: 528b729be1a2 ("gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table")
>
> Co-Authored-By: Tom de Vries <tdevries@suse.de>
>
> PR symtab/31268
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31268
> ---
> gdb/symtab.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 84 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 86603dfebc3..e032178aaa6 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -4156,6 +4156,9 @@ find_epilogue_using_linetable (CORE_ADDR func_addr)
> if (!find_pc_partial_function (func_addr, nullptr, &start_pc, &end_pc))
> return {};
>
> + /* While the standard allows for multiple points marked with epilogue_begin
> + in the same function, for performance reasons, this function will only
> + find the last address that sets this flag for a given block. */
> const struct symtab_and_line sal = find_pc_line (start_pc, 0);
> if (sal.symtab != nullptr && sal.symtab->language () != language_asm)
> {
> @@ -4166,24 +4169,95 @@ find_epilogue_using_linetable (CORE_ADDR func_addr)
> = unrelocated_addr (end_pc - objfile->text_section_offset ());
>
> const linetable *linetable = sal.symtab->linetable ();
> - /* This should find the last linetable entry of the current function.
> - It is probably where the epilogue begins, but since the DWARF 5
> - spec doesn't guarantee it, we iterate backwards through the function
> - until we either find it or are sure that it doesn't exist. */
> + if (linetable == nullptr || linetable->nitems == 0)
> + {
> + /* Empty line table. */
> + return {};
> + }
> +
> + /* Find the first linetable entry after the current function. Note that
> + this also may be an end_sequence entry. */
> auto it = std::lower_bound
> (linetable->item, linetable->item + linetable->nitems, unrel_end,
> [] (const linetable_entry <e, unrelocated_addr pc)
> {
> return lte.unrelocated_pc () < pc;
> });
> + if (it == linetable->item + linetable->nitems)
> + {
> + /* We couldn't find either:
> + - a linetable entry starting the function after the current
> + function, or
> + - an end_sequence entry that terminates the current function
> + at unrel_end.
> +
> + This can happen when the linetable doesn't describe the full
> + extent of the function. This can be triggered with:
> + - compiler-generated debug info, in the cornercase that the pc
> + with which we call find_pc_line resides in a different file
> + than unrel_end, or
> + - invalid dwarf assembly debug info.
> + In the former case, there's no point in iterating further, simply
> + return "not found". In the latter case, there's no current
> + incentive to attempt to support this, so handle this
> + conservatively and do the same. */
> + return {};
> + }
>
> - while (it->unrelocated_pc () >= unrel_start)
> - {
> - if (it->epilogue_begin)
> - return {it->pc (objfile)};
> - it --;
> - }
> + if (unrel_end < it->unrelocated_pc ())
> + {
> + /* We found a line entry that starts past the end of the
> + function. This can happen if the previous entry straddles
> + two functions, which shouldn't happen with compiler-generated
> + debug info. Handle the corner case conservatively. */
> + return {};
> + }
> + gdb_assert (unrel_end == it->unrelocated_pc ());
> +
> + /* Move to the last linetable entry of the current function. */
> + if (it == &linetable->item[0])
> + {
> + /* Doing it-- would introduce undefined behaviour, avoid it by
> + explicitly handling this case. */
> + return {};
> + }
> + it--;
> + if (it->unrelocated_pc () < unrel_start)
> + {
> + /* Not in the current function. */
> + return {};
> + }
> + gdb_assert (it->unrelocated_pc () < unrel_end);
> +
> + /* We're at the the last linetable entry of the current function. This
> + is probably where the epilogue begins, but since the DWARF 5 spec
> + doesn't guarantee it, we iterate backwards through the current
> + function until we either find the epilogue beginning, or are sure
> + that it doesn't exist. */
> + for (; it >= &linetable->item[0]; it--)
> + {
> + if (it->unrelocated_pc () < unrel_start)
> + {
> + /* No longer in the current function. */
> + break;
> + }
> +
> + if (it->epilogue_begin)
> + {
> + /* Found the beginning of the epilogue. */
> + return {it->pc (objfile)};
> + }
> +
> + if (it == &linetable->item[0])
> + {
> + /* No more entries in the current function.
> + Doing it-- would introduce undefined behaviour, avoid it by
> + explicitly handling this case. */
> + break;
> + }
> + }
> }
> +
> return {};
> }
>
>
> base-commit: 9132c8152b899a1683bc886f8ba76bedadb48aa1
The patch is OK from my side.
Thanks
Bernd.
Tom de Vries <tdevries@suse.de> writes:
> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
>
> An out of bounds array access in find_epilogue_using_linetable causes random
> test failures like these:
>
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $fba_value == $fn_fba
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: check frame-id matches
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: bt 2
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: up
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $sp_value == $::main_sp
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $fba_value == $::main_fba
> FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: [string equal $fid $::main_fid]
>
> Here the read happens below the first element of the line
> table, and the test failure depends on the value that is
> read from there.
>
> It also happens that std::lower_bound returns a pointer exactly at the upper
> bound of the line table, also here the read value is undefined, that happens
> in this test:
>
> FAIL: gdb.dwarf2/dw2-epilogue-begin.exp: confirm watchpoint doesn't trigger
>
> Fixes: 528b729be1a2 ("gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table")
>
> Co-Authored-By: Tom de Vries <tdevries@suse.de>
LGTM.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> PR symtab/31268
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31268
> ---
> gdb/symtab.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 84 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 86603dfebc3..e032178aaa6 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -4156,6 +4156,9 @@ find_epilogue_using_linetable (CORE_ADDR func_addr)
> if (!find_pc_partial_function (func_addr, nullptr, &start_pc, &end_pc))
> return {};
>
> + /* While the standard allows for multiple points marked with epilogue_begin
> + in the same function, for performance reasons, this function will only
> + find the last address that sets this flag for a given block. */
> const struct symtab_and_line sal = find_pc_line (start_pc, 0);
> if (sal.symtab != nullptr && sal.symtab->language () != language_asm)
> {
> @@ -4166,24 +4169,95 @@ find_epilogue_using_linetable (CORE_ADDR func_addr)
> = unrelocated_addr (end_pc - objfile->text_section_offset ());
>
> const linetable *linetable = sal.symtab->linetable ();
> - /* This should find the last linetable entry of the current function.
> - It is probably where the epilogue begins, but since the DWARF 5
> - spec doesn't guarantee it, we iterate backwards through the function
> - until we either find it or are sure that it doesn't exist. */
> + if (linetable == nullptr || linetable->nitems == 0)
> + {
> + /* Empty line table. */
> + return {};
> + }
> +
> + /* Find the first linetable entry after the current function. Note that
> + this also may be an end_sequence entry. */
> auto it = std::lower_bound
> (linetable->item, linetable->item + linetable->nitems, unrel_end,
> [] (const linetable_entry <e, unrelocated_addr pc)
> {
> return lte.unrelocated_pc () < pc;
> });
> + if (it == linetable->item + linetable->nitems)
> + {
> + /* We couldn't find either:
> + - a linetable entry starting the function after the current
> + function, or
> + - an end_sequence entry that terminates the current function
> + at unrel_end.
> +
> + This can happen when the linetable doesn't describe the full
> + extent of the function. This can be triggered with:
> + - compiler-generated debug info, in the cornercase that the pc
> + with which we call find_pc_line resides in a different file
> + than unrel_end, or
> + - invalid dwarf assembly debug info.
> + In the former case, there's no point in iterating further, simply
> + return "not found". In the latter case, there's no current
> + incentive to attempt to support this, so handle this
> + conservatively and do the same. */
> + return {};
> + }
>
> - while (it->unrelocated_pc () >= unrel_start)
> - {
> - if (it->epilogue_begin)
> - return {it->pc (objfile)};
> - it --;
> - }
> + if (unrel_end < it->unrelocated_pc ())
> + {
> + /* We found a line entry that starts past the end of the
> + function. This can happen if the previous entry straddles
> + two functions, which shouldn't happen with compiler-generated
> + debug info. Handle the corner case conservatively. */
> + return {};
> + }
> + gdb_assert (unrel_end == it->unrelocated_pc ());
> +
> + /* Move to the last linetable entry of the current function. */
> + if (it == &linetable->item[0])
> + {
> + /* Doing it-- would introduce undefined behaviour, avoid it by
> + explicitly handling this case. */
> + return {};
> + }
> + it--;
> + if (it->unrelocated_pc () < unrel_start)
> + {
> + /* Not in the current function. */
> + return {};
> + }
> + gdb_assert (it->unrelocated_pc () < unrel_end);
> +
> + /* We're at the the last linetable entry of the current function. This
> + is probably where the epilogue begins, but since the DWARF 5 spec
> + doesn't guarantee it, we iterate backwards through the current
> + function until we either find the epilogue beginning, or are sure
> + that it doesn't exist. */
> + for (; it >= &linetable->item[0]; it--)
> + {
> + if (it->unrelocated_pc () < unrel_start)
> + {
> + /* No longer in the current function. */
> + break;
> + }
> +
> + if (it->epilogue_begin)
> + {
> + /* Found the beginning of the epilogue. */
> + return {it->pc (objfile)};
> + }
> +
> + if (it == &linetable->item[0])
> + {
> + /* No more entries in the current function.
> + Doing it-- would introduce undefined behaviour, avoid it by
> + explicitly handling this case. */
> + break;
> + }
> + }
> }
> +
> return {};
> }
>
>
> base-commit: 9132c8152b899a1683bc886f8ba76bedadb48aa1
> --
> 2.35.3
@@ -4156,6 +4156,9 @@ find_epilogue_using_linetable (CORE_ADDR func_addr)
if (!find_pc_partial_function (func_addr, nullptr, &start_pc, &end_pc))
return {};
+ /* While the standard allows for multiple points marked with epilogue_begin
+ in the same function, for performance reasons, this function will only
+ find the last address that sets this flag for a given block. */
const struct symtab_and_line sal = find_pc_line (start_pc, 0);
if (sal.symtab != nullptr && sal.symtab->language () != language_asm)
{
@@ -4166,24 +4169,95 @@ find_epilogue_using_linetable (CORE_ADDR func_addr)
= unrelocated_addr (end_pc - objfile->text_section_offset ());
const linetable *linetable = sal.symtab->linetable ();
- /* This should find the last linetable entry of the current function.
- It is probably where the epilogue begins, but since the DWARF 5
- spec doesn't guarantee it, we iterate backwards through the function
- until we either find it or are sure that it doesn't exist. */
+ if (linetable == nullptr || linetable->nitems == 0)
+ {
+ /* Empty line table. */
+ return {};
+ }
+
+ /* Find the first linetable entry after the current function. Note that
+ this also may be an end_sequence entry. */
auto it = std::lower_bound
(linetable->item, linetable->item + linetable->nitems, unrel_end,
[] (const linetable_entry <e, unrelocated_addr pc)
{
return lte.unrelocated_pc () < pc;
});
+ if (it == linetable->item + linetable->nitems)
+ {
+ /* We couldn't find either:
+ - a linetable entry starting the function after the current
+ function, or
+ - an end_sequence entry that terminates the current function
+ at unrel_end.
+
+ This can happen when the linetable doesn't describe the full
+ extent of the function. This can be triggered with:
+ - compiler-generated debug info, in the cornercase that the pc
+ with which we call find_pc_line resides in a different file
+ than unrel_end, or
+ - invalid dwarf assembly debug info.
+ In the former case, there's no point in iterating further, simply
+ return "not found". In the latter case, there's no current
+ incentive to attempt to support this, so handle this
+ conservatively and do the same. */
+ return {};
+ }
- while (it->unrelocated_pc () >= unrel_start)
- {
- if (it->epilogue_begin)
- return {it->pc (objfile)};
- it --;
- }
+ if (unrel_end < it->unrelocated_pc ())
+ {
+ /* We found a line entry that starts past the end of the
+ function. This can happen if the previous entry straddles
+ two functions, which shouldn't happen with compiler-generated
+ debug info. Handle the corner case conservatively. */
+ return {};
+ }
+ gdb_assert (unrel_end == it->unrelocated_pc ());
+
+ /* Move to the last linetable entry of the current function. */
+ if (it == &linetable->item[0])
+ {
+ /* Doing it-- would introduce undefined behaviour, avoid it by
+ explicitly handling this case. */
+ return {};
+ }
+ it--;
+ if (it->unrelocated_pc () < unrel_start)
+ {
+ /* Not in the current function. */
+ return {};
+ }
+ gdb_assert (it->unrelocated_pc () < unrel_end);
+
+ /* We're at the the last linetable entry of the current function. This
+ is probably where the epilogue begins, but since the DWARF 5 spec
+ doesn't guarantee it, we iterate backwards through the current
+ function until we either find the epilogue beginning, or are sure
+ that it doesn't exist. */
+ for (; it >= &linetable->item[0]; it--)
+ {
+ if (it->unrelocated_pc () < unrel_start)
+ {
+ /* No longer in the current function. */
+ break;
+ }
+
+ if (it->epilogue_begin)
+ {
+ /* Found the beginning of the epilogue. */
+ return {it->pc (objfile)};
+ }
+
+ if (it == &linetable->item[0])
+ {
+ /* No more entries in the current function.
+ Doing it-- would introduce undefined behaviour, avoid it by
+ explicitly handling this case. */
+ break;
+ }
+ }
}
+
return {};
}