[Bug libdw/22546] dwarf_aggregate_size() doesn't work for multi-dimensional arrays

[sourceware at dima dot secretsauce.net]

https://sourceware.org/bugzilla/show_bug.cgi?id=22546

--- Comment #7 from dima kogan  ---
Thanks for looking at it. I submitted the patch.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Re: [PATCH] Fix 22546: dwarf_aggregate_size() works with multi-dimensional arrays

[Mark Wielaard]

On Mon, 2017-12-11 at 21:57 -0800, Dima Kogan wrote:
> If we have a multidimensional array of dimensions (a,b,c) the number of 
> elements
> should be a*b*c, but prior to this patch dwarf_aggregate_size() would report
> a+b+c instead.
> 
> This patch fixes the bug and adds a test that demonstrates the bug (the test
> fails without the functional part of this patch).
> 
> Fixes: https://sourceware.org/bugzilla/show_bug.cgi?id=22546

Thanks. As said in the bug report, this looks good.
Pushed to master.

Cheers,

Mark

[PATCH] libdw: dwarf_aggregate_size should not don't peel the given DIE.

[Mark Wielaard]

Reserve memory for a new DIE first. The caller might not care, but it
isn't really nice to change the DIE the caller gave us.

See also https://sourceware.org/bugzilla/show_bug.cgi?id=22546#c5

Signed-off-by: Mark Wielaard 
---
 libdw/ChangeLog  | 5 +
 libdw/dwarf_aggregate_size.c | 6 +++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 2a6d711..350230e 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,8 @@
+2017-12-12  Mark Wielaard  
+
+   * dwarf_aggregate_size.c (dwarf_aggregate_size): Don't peel the
+   given DIE. Reserve memory for a new DIE first.
+
 2017-12-11  Dima Kogan  
 
* dwarf_aggregate_size.c (array_size): Handle multi-dimensional
diff --git a/libdw/dwarf_aggregate_size.c b/libdw/dwarf_aggregate_size.c
index 3010c0a..6e50185 100644
--- a/libdw/dwarf_aggregate_size.c
+++ b/libdw/dwarf_aggregate_size.c
@@ -199,12 +199,12 @@ aggregate_size (Dwarf_Die *die, Dwarf_Word *size, 
Dwarf_Die *type_mem)
 int
 dwarf_aggregate_size (Dwarf_Die *die, Dwarf_Word *size)
 {
-  Dwarf_Die type_mem;
+  Dwarf_Die die_mem, type_mem;
 
-  if (INTUSE (dwarf_peel_type) (die, die) != 0)
+  if (INTUSE (dwarf_peel_type) (die, &die_mem) != 0)
 return -1;
 
-  return aggregate_size (die, size, &type_mem);
+  return aggregate_size (&die_mem, size, &type_mem);
 }
 INTDEF (dwarf_aggregate_size)
 OLD_VERSION (dwarf_aggregate_size, ELFUTILS_0.144)
-- 
1.8.3.1

[Bug libdw/22546] dwarf_aggregate_size() doesn't work for multi-dimensional arrays

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=22546

Mark Wielaard  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |FIXED

--- Comment #8 from Mark Wielaard  ---
Fix has been pushed to master:
https://sourceware.org/ml/elfutils-devel/2017-q4/msg00103.html

A patch for the separate issue mentioned in comment #5 has been submitted:
https://sourceware.org/ml/elfutils-devel/2017-q4/msg00106.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Re: [PATCH] config: Add pkgincludedir to Cflags

[Namhyung Kim]

Hello,

On Tue, Dec 12, 2017 at 8:13 AM, Mark Wielaard  wrote:
> On Fri, 2017-12-08 at 15:17 +0900, Namhyung Kim wrote:
>> The libelf and libdw uses pkginclude_HEADERS but the pkg-config file
>> misses to have it the Cflags.  This was a problem for libdw users since
>> it gave nothing to include, but one needs to add /usr/include/elfutils
>> directory to use the libdw.h header file.
>
> I this really an issue?
>
> libelf has two public headers which are placed in the includedir and so
> should be simply included as #include  and #include .
>
> libdw does indeed put its headers in includdir/elfutils, but as far as
> I know everybody includes them using that subdir, so #include
>  and #include , etc.
>
> Wouldn't this change how people include the headers?
> Do we want to change it?

Oh, I wasn't aware of the (preferred) way of including libdw headers.
Will change my code to use that pattern.  Sorry for the noise.

Thanks,
Namhyung