Hi,
On Fri, 2024-03-01 at 15:59 +0100, Mark Wielaard wrote:
> This looks correct, but gcc noticed a path to use tu_offset (and
> tu_index) if they weren't initialized or NULL:
>
> In file included from /home/mark/src/elfutils/libdw/libdwP.h:684,
> from
> /home/mark/src/elfutils/libdw/dwarf_cu_dwp_section_info.c:35:
> In function ‘read_4ubyte_unaligned_1’,
> inlined from ‘__libdw_package_index’ at
> /home/mark/src/elfutils/libdw/dwarf_cu_dwp_section_info.c:302:34:
> /home/mark/src/elfutils/libdw/memory-access.h:291:12: error: ‘tu_offset’ may
> be used uninitialized [-Werror=maybe-uninitialized]
> 291 | return up->u4;
> | ~~^~~~
> /home/mark/src/elfutils/libdw/dwarf_cu_dwp_section_info.c: In function
> ‘__libdw_package_index’:
> /home/mark/src/elfutils/libdw/dwarf_cu_dwp_section_info.c:268:28: note:
> ‘tu_offset’ was declared here
> 268 | const unsigned char *tu_offset;
> | ^~~~~~~~~
> cc1: all warnings being treated as errors
>
> I couldn't immediately disprove gcc here, so I think it is a good idea
> to add an explicit check for tu_index != NULL.
>
> diff --git a/libdw/dwarf_cu_dwp_section_info.c
> b/libdw/dwarf_cu_dwp_section_info.c
> index 3d11c87a..9fdc15bf 100644
> --- a/libdw/dwarf_cu_dwp_section_info.c
> +++ b/libdw/dwarf_cu_dwp_section_info.c
> @@ -297,7 +297,8 @@ __libdw_package_index (Dwarf *dbg, bool tu)
> cu_index->debug_info_offsets[cui++] = off;
> cu_offset += cu_index->section_count * 4;
> }
> - else if (unit_type == DW_UT_split_type && tui < tu_count)
> + else if (unit_type == DW_UT_split_type && tu_index != NULL
> + && tui < tu_count)
> {
> if ((off & UINT32_MAX) != read_4ubyte_unaligned (dbg,
> tu_offset))
> goto not_sorted;
>
> Which makes gcc happy again.
But not all gcc versions apparently. So I added the following on top.
From cc6e53b9f305148bda275ade40c0e625d98da2f2 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Fri, 1 Mar 2024 17:05:16 +0100
Subject: [PATCH] libdw: Initialize tu_offset in __libdw_package_index
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
dwarf_cu_dwp_section_info.c: In function ‘__libdw_package_index’:
dwarf_cu_dwp_section_info.c:306:25: error: ‘tu_offset’ may be used uninitialized [-Werror=maybe-uninitialized]
306 | tu_offset += tu_index->section_count * 4;
| ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
dwarf_cu_dwp_section_info.c:268:28: note: ‘tu_offset’ was declared here
268 | const unsigned char *tu_offset;
| ^~~~~~~~~
Which is the same issue we thought to have fixed by checking for
tu_index != NULL but not all gcc versions seem able to see that.
So just explicitly initialize tu_offset to NULL. We keep the older
check, so the NULL pointer should never be used.
* libdw/dwarf_cu_dwp_section_info.c (__libdw_package_index):
Initialize tu_offset.
Signed-off-by: Mark Wielaard <m...@klomp.org>
---
libdw/dwarf_cu_dwp_section_info.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libdw/dwarf_cu_dwp_section_info.c b/libdw/dwarf_cu_dwp_section_info.c
index 9fdc15bf..5a081f5a 100644
--- a/libdw/dwarf_cu_dwp_section_info.c
+++ b/libdw/dwarf_cu_dwp_section_info.c
@@ -265,7 +265,7 @@ __libdw_package_index (Dwarf *dbg, bool tu)
const unsigned char *cu_offset
= cu_index->section_offsets + cu_index->sections[DW_SECT_INFO - 1] * 4;
uint32_t tu_count = 0;
- const unsigned char *tu_offset;
+ const unsigned char *tu_offset = NULL;
if (tu_index != NULL)
{
tu_count = tu_index->unit_count;
--
2.43.2
