Re: dwarf_aggregate_size() seems to fall over on pointer types

[Mark Wielaard <mjw@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 547 bytes --]

On Wed, 2014-10-01 at 15:26 -0400, Conrad Rad wrote:
> On Wed, Oct 1, 2014 at 3:23 PM, Conrad Rad <cse.cem@gmail.com> wrote:
> > This should be 0, 0x8. The size goes in the *size, return value of
> > dwarf_aggregate_size() should be zero on success, not 8. The patch is
> > not good as is.
> >
> With the fixed patch (attached), it works:

Oops. That shows why I shouldn't submit patches without testcases...
Thanks for testing and fixing.

Here is the fixed up patch plus some testcases to show it actually
works.

Cheers,

Mark

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-libdw-dwarf_aggregate_size-return-CU-address_size-fo.patch --]
[-- Type: text/x-patch, Size: 11987 bytes --]

From 8b75c34913ced8257b57397043380bc4c23d7067 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Thu, 2 Oct 2014 14:00:47 +0200
Subject: [PATCH] libdw: dwarf_aggregate_size return CU address_size for
 sizeless pointer/refs.

Tested-by: Conrad Rad <cse.cem@gmail.com>
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libdw/ChangeLog              |   6 ++++
 libdw/dwarf_aggregate_size.c |  10 +++++-
 tests/ChangeLog              |  10 ++++++
 tests/Makefile.am            |   5 +--
 tests/aggregate_size.c       |  83 +++++++++++++++++++++++++++++++++++++++++++
 tests/run-aggregate-size.sh  |  66 ++++++++++++++++++++++++++++++++++
 tests/testfile-sizes1.o.bz2  | Bin 0 -> 1012 bytes
 tests/testfile-sizes2.o.bz2  | Bin 0 -> 1283 bytes
 8 files changed, 177 insertions(+), 3 deletions(-)
 create mode 100644 tests/aggregate_size.c
 create mode 100755 tests/run-aggregate-size.sh
 create mode 100644 tests/testfile-sizes1.o.bz2
 create mode 100644 tests/testfile-sizes2.o.bz2

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 410b31a..29edc46 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,9 @@
+2014-10-02  Mark Wielaard  <mjw@redhat.com>
+
+	* dwarf_aggregate_size.c (aggregate_size): Return CU address_size
+	for sizeless DW_TAG_pointer_type, DW_TAG_reference_type or
+	DW_TAG_rvalue_reference_type.
+
 2014-09-12  Petr Machata  <pmachata@redhat.com>
 
 	* memory-access.h (read_ubyte_unaligned_inc): Allow only 4- and
diff --git a/libdw/dwarf_aggregate_size.c b/libdw/dwarf_aggregate_size.c
index 07c53a2..5d23541 100644
--- a/libdw/dwarf_aggregate_size.c
+++ b/libdw/dwarf_aggregate_size.c
@@ -1,5 +1,5 @@
 /* Compute size of an aggregate type from DWARF.
-   Copyright (C) 2010 Red Hat, Inc.
+   Copyright (C) 2010, 2014 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -205,6 +205,14 @@ aggregate_size (Dwarf_Die *die, Dwarf_Word *size, Dwarf_Die *type_mem)
 
     case DW_TAG_array_type:
       return array_size (die, size, &attr_mem, type_mem);
+
+    /* Assume references and pointers have pointer size if not given an
+       explicit DW_AT_byte_size.  */
+    case DW_TAG_pointer_type:
+    case DW_TAG_reference_type:
+    case DW_TAG_rvalue_reference_type:
+      *size = die->cu->address_size;
+      return 0;
     }
 
   /* Most types must give their size directly.  */
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 96320fa..b691007 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,13 @@
+2014-10-02  Mark Wielaard  <mjw@redhat.com>
+
+	* Makefile.am (check_PROGRAMS): Add aggregate_size.c.
+	(TESTS, EXTRA_DIST): Add run-aggregate-size.sh.
+	(aggregate_size_LDADD): New variable.
+	* aggregate_size.c: New file.
+	* run-aggregate-size.sh: New test.
+	* testfile-sizes1.o.bz2: New test file.
+	* testfile-sizes2.o.bz2: Likewise.
+
 2014-09-26  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	Support NT_FILE for locating files.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ebf4f71..133501f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -50,7 +50,7 @@ check_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \
 		  test-elf_cntl_gelf_getshdr dwflsyms dwfllines \
 		  dwfl-report-elf-align varlocs backtrace backtrace-child \
 		  backtrace-data backtrace-dwarf debuglink debugaltlink \
-		  buildid deleted deleted-lib.so
+		  buildid deleted deleted-lib.so aggregate_size
 
 asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \
 	    asm-tst6 asm-tst7 asm-tst8 asm-tst9
@@ -110,7 +110,7 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \
 	run-backtrace-core-aarch64.sh \
 	run-backtrace-demangle.sh run-stack-d-test.sh run-stack-i-test.sh \
 	run-readelf-dwz-multi.sh run-allfcts-multi.sh run-deleted.sh \
-	run-linkmap-cut.sh
+	run-linkmap-cut.sh run-aggregate-size.sh
 
 if !BIARCH
 export ELFUTILS_DISABLE_BIARCH = 1
@@ -412,6 +412,7 @@ buildid_LDADD = $(libdw) $(libelf)
 deleted_LDADD = ./deleted-lib.so
 deleted_lib_so_LDFLAGS = -shared -rdynamic
 deleted_lib_so_CFLAGS = -fPIC
+aggregate_size_LDADD = $(libdw) $(libelf)
 
 if GCOV
 check: check-am coverage
diff --git a/tests/aggregate_size.c b/tests/aggregate_size.c
new file mode 100644
index 0000000..930eafa
--- /dev/null
+++ b/tests/aggregate_size.c
@@ -0,0 +1,83 @@
+/* Test program for dwarf_aggregate_size. Prints size of top-level vars.
+   Copyright (C) 2014 Red Hat, Inc.
+   This file is part of elfutils.
+
+   This file is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   elfutils is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include <assert.h>
+#include <argp.h>
+#include <inttypes.h>
+#include <fcntl.h>
+#include ELFUTILS_HEADER(dw)
+#include ELFUTILS_HEADER(dwfl)
+#include <stdio.h>
+#include <unistd.h>
+#include <dwarf.h>
+
+void
+print_var_type_size (Dwarf_Die *var)
+{
+  Dwarf_Attribute attr_mem;
+  Dwarf_Die type_mem;
+  Dwarf_Die *type;
+  const char *name = dwarf_diename (var);
+
+  type = dwarf_formref_die (dwarf_attr (var, DW_AT_type, &attr_mem),
+			    &type_mem);
+  if (type != NULL)
+    {
+      Dwarf_Word size;
+      if (dwarf_aggregate_size (type, &size) < 0)
+        printf ("%s no size: %s\n", name, dwarf_errmsg (-1));
+      else
+	printf ("%s size %" PRIu64 "\n", name, size);
+    }
+  else
+    printf ("%s has no type.\n", name);
+}
+
+int
+main (int argc, char *argv[])
+{
+
+  int remaining;
+  Dwfl *dwfl;
+  (void) argp_parse (dwfl_standard_argp (), argc, argv, 0, &remaining,
+                     &dwfl);
+  assert (dwfl != NULL);
+
+  Dwarf_Die *cu = NULL;
+  Dwarf_Addr dwbias;
+  while ((cu = dwfl_nextcu (dwfl, cu, &dwbias)) != NULL)
+    {
+      Dwarf_Die die_mem;
+      Dwarf_Die *die = &die_mem;
+      dwarf_child (cu, &die_mem);
+
+      while (1)
+	{
+	  if (dwarf_tag (die) == DW_TAG_variable)
+	    print_var_type_size (die);
+
+	  if (dwarf_siblingof (die, &die_mem) != 0)
+	    break;
+	}
+    }
+
+  dwfl_end (dwfl);
+}
diff --git a/tests/run-aggregate-size.sh b/tests/run-aggregate-size.sh
new file mode 100755
index 0000000..82a5d53
--- /dev/null
+++ b/tests/run-aggregate-size.sh
@@ -0,0 +1,66 @@
+#! /bin/sh
+# Copyright (C) 2014 Red Hat, Inc.
+# This file is part of elfutils.
+#
+# This file is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# elfutils is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. $srcdir/test-subr.sh
+
+# char c;
+# int i;
+# long l;
+#
+# void *v;
+#
+# struct s
+# {
+#   char *a;
+#   int i;
+# } s;
+#
+# char ca[16];
+# int ia[32];
+# void *va[64];
+# struct s sa[8];
+
+# gcc -g -c -o testfile-sizes1.o sizes.c
+# clang -g -c -o testfile-sizes2.o sizes.c
+
+testfiles testfile-sizes1.o testfile-sizes2.o
+
+testrun_compare ${abs_builddir}/aggregate_size -e testfile-sizes1.o <<\EOF
+c size 1
+i size 4
+l size 8
+v size 8
+s size 16
+ca size 16
+ia size 128
+va size 512
+sa size 128
+EOF
+
+testrun_compare ${abs_builddir}/aggregate_size -e testfile-sizes2.o <<\EOF
+c size 1
+i size 4
+l size 8
+v size 8
+s size 16
+ca size 16
+ia size 128
+va size 512
+sa size 128
+EOF
+
+exit 0
diff --git a/tests/testfile-sizes1.o.bz2 b/tests/testfile-sizes1.o.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..479ecb20149dfd1a2520aead7fe3562e77b6681b
GIT binary patch
literal 1012
zcmV<Q0}K2@T4*^jL0KkKSvxE!LjVJ_|NsC0|L^Ya^<UrZ(h0xs-|*rjz(@!HKtKRU
z1PBNKI7rX~L%>KYZHzP_G$thvH5!l9^rw<~8Bb4C(@mt)6DC7LY5)&VWN5_GY3gYW
z10fNqsp+PgMDk?B37{UJ0009(0000000E$I00000000I<KmY&$000008~^|S00001
zkk9}C0000000KlwsivvoX{Mtln@EO+CPtV?qJE&7G&DU;G#X@K0j6kW7(s_|uDZrJ
z?_<WWB4j2?I28KJUiR$xIQeMP7_`SqUOj0zY;&kTLb%36243z4OA=LlbqA^BlmPP)
z!?hh-FIsG8V<OF{=}{Bt;HSTzaiUOX9vD|>!OW!q!$X-^>QdI(q^Z@BwaNXgP8;6;
zkLb_Lw`%`6hI}0fNk%_cEd(PwORv-6TR|Fc>v1qs%Ly%9+So1W{uiWMm?`A>JidEz
zf6#<z@*Fadkq;C;A11n-y!pNxgk0NB?0c*hSYw%~lO&{7g6c34g`iT<ue49=3w+Yw
zw1b|-Ru`IX^g!5z8!a1bM`Moy)Y*>)*Ah&0zX2g@LK{N2uLuz>ENn(*64PimA2s_A
zw7@KBCdAkk;|>I+ohAXXiU1C4G&eBq*TXqyXtmN7t5)o9Cwpzus494&RR(kv-nK~q
zk~a3|@00nhjwL5w?k~xngt8+C0kn#!Lf^*t?&W#hiPJ%tV<^Kcp#YICqS_44pAMh)
zb`ZZXKt2YG`}jtViVb;0<j_GVS~kE03`mBxwKSBavczCvm5%Y2Ql=PK#)}wLw_`zZ
zaR{-)hr2ATlw@o6=uw<Tg=MWp5HeVJT>l;Xlmj-wmT@q-g?tH+o#GLdhU+_m^WMqm
z>i;uZ)?)@pENC>#2n(q)VTeI33L~QL#T8*j7hKSe7&9H2-D4lDr5ps5y@OLTR`@ye
z#~0ja(Yb}pw{Fyp>C@RP1}xRmDPpp|$ZrFoq0voqhJ%Nz7IrHp6498ogKYf(I6<(6
zNPsK=+3A-6`%QpQkg+P-iQj{q3e5SYB;^&C8lsrCwGl(-N*9HS&{2uxBfk+SM*#d1
zIC%ssCN0~mwzV$kS$@xFUVO(q<MBhQt-(XrdQTP#QXT6mF@Ju23^RcqV&kQzbB!V(
zrxQCK#c#p~aKZ|f?8XGDQlY)AU;&y4N?;=cEy){li%85J<ca3ZSjDA5x1eeCgT-Vi
z)U;wqh|9VpWMeQiVi`(#_1t7zxF|MAVJmPnh)e)<#~?if=c7pyzdXrAzyu(mRnZe9
i8HIFhv3m1DWV-I^u!uiZ&Q4MT{x0N-aG@Y}SWt$D(ZqrP

literal 0
HcmV?d00001

diff --git a/tests/testfile-sizes2.o.bz2 b/tests/testfile-sizes2.o.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..7bd7b47f74254dec3f4084abc0467294d7f3b6d6
GIT binary patch
literal 1283
zcmV+e1^oI#T4*^jL0KkKSxt<?MgRqj|NsC0|Ly<l{r~^x+Ts82|9Rk{;4%XhK@4C^
zNEkGM^9axcT3b90R?KqU!-FWMo~fzi%4VjXOoo7)Q_^~RlhkRALjWhJFc1crs2+l5
zgCz8xk&O)ongV%D2~$asOpFmdOsDA(Xahh2lS4o~5NH4Z0000000E!_O%REYr|KuE
z>UmFUAZQu`LqIeEqd*NFkN^Mx0017Qk5B+;g8)Veh|?w!ki;;ICIChekjbV)Mn)je
zz)VaDh|z$InJ@_wgr~Ao+L5MB83uzu0imE|WB@b(0000041fRupcvA<9{7|9$`Zyt
z1_hT6bPy=2G3zKaUd-t#y)KtaX5D@l-qIc{nGPeyir%PGfn{9x!E5Hh+sNpL7BroS
zQ#xy$gm%Sz=b_TTvGp~^R7|-!wU66=S$EN(*hf~V{VAk|mlmwhLQnz?yda0@gMsC|
z!NP5{&ce~UN)g2fWGUA3aQMAneoGVYhb`^sotUKJ!i|&F<O0&)+G}50#9A#CIw{?%
z@R5$Y4%+t@Zw54(I$?;5IH{wQ#y}cdj^!#|)@J)YZ-tuev#@-RWw8@h;OAHtQ#GJ-
z#8Ct}Uz3$!K_gT9oi)c{L>0;dq_AKXoR9%}y6_ix&D*MKhJcV7p+tj<@Cc;4xw*c@
zSY1@=UeJ=DV8#p;Xds-484PIa{w!oLKek&;)WJQcHcbmsmTduG(i*ma7CDI^wP+Nr
zAeoCk)HItAts7#7gaIq<w1Q?OBO60#UOTuIW{i6P)?h*=X$iEFGlfqShy^P^n@x#M
z(1>WsL<4C6*pS*l+8ansiLqa>`ZD!}^A13?y?=*#>|hc{ejzHxmdKK2tZqC@!6_+6
zkfMUCC}NbALknsoRg|D(L=f>tb`OHQ5>he*hW_~gmh7!(>C@wNKYi7~kX)K~GL4y-
zhVdYsNUY#h*qa-L4?ZW#UuFW-r$V4KOO2A_p(4pfAuEzmN*+3~Y7v$L{U@SyB_TnA
z$#8?h%^}hV0g<1RmZ&5J5XHS1C4nTFfSa*k3uSLjdo?m9VN#d`=!FwOJ9mKsA&68e
zAr68iGBHHq2AXSHu~Mwpnkr`(?am}fiX<r7*)VYNV1O}5p9&$!Go}ytqbWp6g)zp*
zt!a>eNqDq~w67S)6sMxP#Y!bW1*UZ5q`}}qV_%-&A}9pUR79Xc2Yp4;Bnu%yC5R&h
zRRSp)z<}T+CCw{?A9ZiNZ2cJFmuv7peSGB1TuBv!R5r1r#6CsF>ePzE5lAAGM&Rj&
z$8j4w3c^V&|3h9^u6hTWOfCoEDsQe?+O>KyQ<PWdA2bqgu!>@J<FG%!pw}_PO)+q?
zB)C|5X_n~1D*jrscxP6*3M&@^CX}|esRrKuYjQ$DE&_nOo|5>iF$D0XZ1+Ie_Pk5r
zLVd^-Wuge62y(JeNTw0VR;Z!?Ym<}<#o;OUk{i$25p;n#p$SR|?7%9e!V9;r-4W#!
zvNoy`$S_C76t1nS53l%xr6{G0424DoO0tsMjnTrA#pYH?$cX4c%c(+kOgcRR4g<>=
zgNn9RaZpe!uS8a>Sbh|iY;+5<doh#XjgF-)2`0T35Khw+7eK5lRdnd8Dg_sJ^Roxu
tK~8DmoLu#E8DNU$CCsH+!H$>+J?MfNjc5BA08sz<yOJrwgo12l8Ze>lK<fYi

literal 0
HcmV?d00001

-- 
1.8.3.1