Hi Aaron,
On Fri, 2023-11-17 at 21:48 -0500, Aaron Merey wrote:
> v1: https://sourceware.org/pipermail/elfutils-devel/2023q4/006644.html
>
> v2 changes:
>
> The size of the uncompressed testcore-noncontig has been reduced from
> 736M to 54K.
Uncompressed it is 580K. Still a bit large, but much more reasonable.
We even have a couple of larger test files in the repo. Thanks.
And in theory it can be replicated.
> dwfl_addrsegment tests have been added to verify correct handling of
> non-contiguous segments.
BTW. Adding extra comments after the --- makes it easier to post a
commit as you will apply it because comments after the --- will be
ignored by git am.
Please restore the original commit message before pushing.
The description of the issue was really good.
> ---
> libdwfl/dwfl_segment_report_module.c | 37 ++-
> tests/.gitignore | 1 +
> tests/Makefile.am| 8 ++--
> tests/dwfl-core-noncontig.c | 67 +++
> tests/run-dwfl-core-noncontig.sh | 63 +
> tests/testcore-noncontig.bz2 | Bin 0 -> 54684 bytes
> 6 files changed, 162 insertions(+), 14 deletions(-)
> create mode 100644 tests/dwfl-core-noncontig.c
> create mode 100755 tests/run-dwfl-core-noncontig.sh
> create mode 100644 tests/testcore-noncontig.bz2
>
> diff --git a/libdwfl/dwfl_segment_report_module.c
> b/libdwfl/dwfl_segment_report_module.c
> index 3ef62a7d..09ee37b3 100644
> --- a/libdwfl/dwfl_segment_report_module.c
> +++ b/libdwfl/dwfl_segment_report_module.c
> @@ -737,17 +737,34 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
> char *name,
> && invalid_elf (module->elf, module->disk_file_has_build_id,
> _id))
> {
> - elf_end (module->elf);
> - close (module->fd);
> - module->elf = NULL;
> - module->fd = -1;
> + /* If MODULE's build-id doesn't match the disk file's
> +build-id, close ELF only if MODULE and ELF refer to
> +different builds of files with the same name. This
> +prevents premature closure of the correct ELF in cases
> +where segments of a module are non-contiguous in memory. */
> + if (name != NULL && module->name[0] != '\0'
> + && strcmp (basename (module->name), basename (name)) == 0)
> + {
> + elf_end (module->elf);
> + close (module->fd);
> + module->elf = NULL;
> + module->fd = -1;
> + }
> }
> - if (module->elf != NULL)
> + else if (module->elf != NULL)
> {
> - /* Ignore this found module if it would conflict in address
> -space with any already existing module of DWFL. */
> + /* This module has already been reported. */
> skip_this_module = true;
> }
> + else
> + {
> + /* Only report this module if we haven't already done so. */
> + for (Dwfl_Module *mod = dwfl->modulelist; mod != NULL;
> + mod = mod->next)
> + if (mod->low_addr == module_start
> + && mod->high_addr == module_end)
> + skip_this_module = true;
> + }
> }
>if (skip_this_module)
> goto out;
> @@ -781,10 +798,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
> char *name,
> }
> }
>
> - /* Our return value now says to skip the segments contained
> - within the module. */
> - ndx = addr_segndx (dwfl, segment, module_end, true);
> -
>/* Examine its .dynamic section to get more interesting details.
> If it has DT_SONAME, we'll use that as the module name.
> If it has a DT_DEBUG, then it's actually a PIE rather than a DSO.
> @@ -929,6 +942,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
> char *name,
>ndx = -1;
>goto out;
> }
> + else
> +ndx++;
>
>/* We have reported the module. Now let the caller decide whether we
> should read the whole thing in right now. */
Code looks the same.
> diff --git a/tests/.gitignore b/tests/.gitignore
> index b9aa22ba..5bebb2c4 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -49,6 +49,7 @@
> /dwfl-report-elf-align
> /dwfl-report-offline-memory
> /dwfl-report-segment-contiguous
> +/dwfl-core-noncontig
> /dwfllines
> /dwflmodtest
> /dwflsyms
Ack.
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 7fb8efb1..9f8f7698 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -42,7 +42,7 @@ check_PROGRAMS = arextract arsymtest newfile saridx
> scnnames sectiondump \
> dwfl-bug-addr-overflow arls dwfl-bug-fd-leak \
> dwfl-addr-sect dwfl-bug-report early-offscn \
>
