public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] riscv: Partial implementation of flatten_aggregate
@ 2024-03-20 15:03 Mark Wielaard
  2024-03-20 18:14 ` Aaron Merey
  2024-03-20 20:17 ` Palmer Dabbelt
  0 siblings, 2 replies; 6+ messages in thread
From: Mark Wielaard @ 2024-03-20 15:03 UTC (permalink / raw)
  To: elfutils-devel; +Cc: Mark Wielaard

dwfl_module_return_value_location would fail on riscv for functions
which return a (small) struct. This patch implements the simplest
cases of flatten_aggregate in backends/riscv_retval.c. It just handles
structs containing one or two members of the same base type which fit
completely or in pieces in one or two general or floating point
registers.

It also adds a specific test case run-funcretval-struct.sh containing
small structs of ints, longs, floats and doubles. All these testscases
now work for riscv. There is already a slightly more extensive
testcase for this in tests/run-funcretval.sh but that only has a
testcase for aarch64.

	 * backends/riscv_retval.c (flatten_aggregate_arg): Implement
         for the simple cases where we have a struct with one or two
         members of the same base type.
	 (pass_by_flattened_arg): Likewise. Call either
	 pass_in_gpr_lp64 or pass_in_fpr_lp64d.
	 (riscv_return_value_location_lp64ifd): Call
	 flatten_aggregate_arg including size.
	 * tests/Makefile.am (TESTS): Add run-funcretval-struct.sh
	 and run-funcretval-struct-native.sh.
	 (check_PROGRAMS): Add funcretval_test_struct.
	 (funcretval_test_struct_SOURCES): New.
	 (EXTRA_DIST): Add run-funcretval-struct.sh,
	 funcretval_test_struct_riscv.bz2 and
	 run-funcretval-struct-native.sh.
	 * tests/funcretval_test_struct_riscv.bz2: New test binary.
	 * tests/run-funcretval-struct-native.sh: New test.
	 * tests/run-funcretval-struct.sh: Likewise.

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

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 backends/riscv_retval.c                | 123 ++++++++++++++++++++++---
 tests/Makefile.am                      |   7 ++
 tests/funcretval_test_struct.c         |  86 +++++++++++++++++
 tests/funcretval_test_struct_riscv.bz2 | Bin 0 -> 3821 bytes
 tests/run-funcretval-struct-native.sh  |  22 +++++
 tests/run-funcretval-struct.sh         |  35 +++++++
 6 files changed, 262 insertions(+), 11 deletions(-)
 create mode 100644 tests/funcretval_test_struct.c
 create mode 100755 tests/funcretval_test_struct_riscv.bz2
 create mode 100755 tests/run-funcretval-struct-native.sh
 create mode 100755 tests/run-funcretval-struct.sh

diff --git a/backends/riscv_retval.c b/backends/riscv_retval.c
index 0a1e02f81cd2..50c451a4ba32 100644
--- a/backends/riscv_retval.c
+++ b/backends/riscv_retval.c
@@ -1,6 +1,7 @@
 /* Function return value location for Linux/RISC-V ABI.
    Copyright (C) 2018 Sifive, Inc.
    Copyright (C) 2013 Red Hat, Inc.
+   Copyright (C) 2024 Mark J. Wielaard <mark@klomp.org>
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -105,23 +106,123 @@ pass_in_fpr_lp64d (const Dwarf_Op **locp, Dwarf_Word size)
   return size <= 8 ? 1 : 4;
 }
 
+/* Checks if we can "flatten" the given type, Only handles the simple
+   cases where we have a struct with one or two the same base type
+   elements.  */
 static int
-flatten_aggregate_arg (Dwarf_Die *typedie __attribute__ ((unused)),
-		       Dwarf_Die *arg0 __attribute__ ((unused)),
-		       Dwarf_Die *arg1 __attribute__ ((unused)))
+flatten_aggregate_arg (Dwarf_Die *typedie,
+		       Dwarf_Word size,
+		       Dwarf_Die *arg0,
+		       Dwarf_Die *arg1)
 {
-  /* ??? */
+  int tag0, tag1;
+  Dwarf_Die member;
+  Dwarf_Word encoding0, encoding1;
+  Dwarf_Attribute attr;
+  Dwarf_Word size0, size1;
+
+  if (size < 8 || size > 16)
+    return 0;
+
+  if (dwarf_child (typedie, arg0) != 0)
+    return 0;
+
+  tag0 = dwarf_tag (arg0);
+  while (tag0 != -1 && tag0 != DW_TAG_member)
+    {
+      if (dwarf_siblingof (arg0, arg0) != 0)
+	return 0;
+      tag0 = dwarf_tag (arg0);
+    }
+
+  if (tag0 != DW_TAG_member)
+    return 0;
+
+  /* Remember where we are.  */
+  member = *arg0;
+
+  tag0 = dwarf_peeled_die_type (arg0, arg0);
+  if (tag0 != DW_TAG_base_type)
+    return 0;
+
+  if (dwarf_attr_integrate (arg0, DW_AT_encoding, &attr) == NULL
+      || dwarf_formudata (&attr, &encoding0) != 0)
+    return 0;
+
+  if (dwarf_bytesize_aux (arg0, &size0) != 0)
+    return 0;
+
+  if (size == size0)
+    return 1; /* This one member is the whole size. */
+
+  if (size != 2 * size0)
+    return 0; /* We only handle two of the same.  */
+
+  /* Look for another member with the same encoding.  */
+  if (dwarf_siblingof (&member, arg1) != 0)
+    return 0;
+
+  tag1 = dwarf_tag (arg1);
+  while (tag1 != -1 && tag1 != DW_TAG_member)
+    {
+      if (dwarf_siblingof (arg1, arg1) != 0)
+	return 0;
+      tag1 = dwarf_tag (arg1);
+    }
+
+  if (tag1 != DW_TAG_member)
+    return 0;
+
+  tag1 = dwarf_peeled_die_type (arg1, arg1);
+  if (tag1 != DW_TAG_base_type)
+    return 0; /* We can only handle two equal base types for now. */
+
+  if (dwarf_attr_integrate (arg1, DW_AT_encoding, &attr) == NULL
+      || dwarf_formudata (&attr, &encoding1) != 0
+      || encoding0 != encoding1)
+    return 0; /* We can only handle two of the same for now. */
+
+  if (dwarf_bytesize_aux (arg1, &size1) != 0)
+    return 0;
+
+  if (size0 != size1)
+    return 0; /* We can only handle two of the same for now. */
+
   return 1;
 }
 
+/* arg0 and arg1 should be the peeled die types found by
+   flatten_aggregate_arg.  */
 static int
-pass_by_flattened_arg (const Dwarf_Op **locp __attribute__ ((unused)),
-		       Dwarf_Word size __attribute__ ((unused)),
-		       Dwarf_Die *arg0 __attribute__ ((unused)),
-		       Dwarf_Die *arg1 __attribute__ ((unused)))
+pass_by_flattened_arg (const Dwarf_Op **locp,
+		       Dwarf_Word size,
+		       Dwarf_Die *arg0,
+		       Dwarf_Die *arg1 __attribute__((unused)))
 {
-  /* ??? */
-  return -2;
+  /* For now we just assume arg0 and arg1 are the same type and
+     encoding.  */
+  Dwarf_Word encoding;
+  Dwarf_Attribute attr;
+
+  if (dwarf_attr_integrate (arg0, DW_AT_encoding, &attr) == NULL
+      || dwarf_formudata (&attr, &encoding) != 0)
+    return -1;
+
+  switch (encoding)
+    {
+    case DW_ATE_boolean:
+    case DW_ATE_signed:
+    case DW_ATE_unsigned:
+    case DW_ATE_unsigned_char:
+    case DW_ATE_signed_char:
+      return pass_in_gpr_lp64 (locp, size);
+
+    case DW_ATE_float:
+      return pass_in_fpr_lp64d (locp, size);
+
+    default:
+      return -1;
+    }
 }
 
 int
@@ -158,7 +259,7 @@ riscv_return_value_location_lp64ifd (int fp, Dwarf_Die *functypedie,
 	 provided the floating-point real is no more than FLEN bits wide and
 	 the integer is no more than XLEN bits wide.  */
       if (tag == DW_TAG_structure_type
-	  && flatten_aggregate_arg (&typedie, &arg0, &arg1))
+	  && flatten_aggregate_arg (&typedie, size, &arg0, &arg1))
 	return pass_by_flattened_arg (locp, size, &arg0, &arg1);
       /* Aggregates larger than 2*XLEN bits are passed by reference.  */
       else if (size > 16)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9141074fe44c..9315ec3bbe4c 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -162,6 +162,7 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh newfile test-nlist \
 	run-addr2line-C-test.sh \
 	run-addr2line-i-test.sh run-addr2line-i-lex-test.sh \
 	run-addr2line-i-demangle-test.sh run-addr2line-alt-debugpath.sh \
+	run-funcretval-struct.sh \
 	run-varlocs.sh run-exprlocs.sh run-varlocs-vars.sh run-funcretval.sh \
 	run-backtrace-native.sh run-backtrace-data.sh run-backtrace-dwarf.sh \
 	run-backtrace-native-biarch.sh run-backtrace-native-core.sh \
@@ -284,6 +285,10 @@ funcretval_test__11_SOURCES = funcretval_test++11.cxx
 TESTS += run-funcretval++11.sh
 endif
 
+check_PROGRAMS += funcretval_test_struct
+funcretval_test_struct_SOURCES = funcretval_test_struct.c
+TESTS += run-funcretval-struct-native.sh
+
 EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
 	     run-ar-N.sh \
 	     run-show-die-info.sh run-get-files.sh run-get-lines.sh \
@@ -478,6 +483,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
 	     testfile_aarch64_core.bz2 testfile_i686_core.bz2 \
 	     addrx_constx-4.dwo.bz2 addrx_constx-5.dwo.bz2 \
 	     testfile-addrx_constx-4.bz2 testfile-addrx_constx-5.bz2 \
+	     run-funcretval-struct.sh funcretval_test_struct_riscv.bz2 \
 	     run-funcretval.sh funcretval_test.c funcretval_test_aarch64.bz2 \
 	     run-backtrace-data.sh run-backtrace-dwarf.sh cleanup-13.c \
 	     run-backtrace-native.sh run-backtrace-native-biarch.sh \
@@ -635,6 +641,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
 	     testfile_nvidia_linemap.bz2 \
 	     testfile-largealign.o.bz2 run-strip-largealign.sh \
 	     run-funcretval++11.sh \
+	     run-funcretval-struct-native.sh \
 	     test-ar-duplicates.a.bz2 \
 	     run-dwfl-core-noncontig.sh testcore-noncontig.bz2 \
 	     testfile-dwarf5-line-clang.bz2 \
diff --git a/tests/funcretval_test_struct.c b/tests/funcretval_test_struct.c
new file mode 100644
index 000000000000..df94bde0a42d
--- /dev/null
+++ b/tests/funcretval_test_struct.c
@@ -0,0 +1,86 @@
+/* Copyright (C) 2024 Mark J. Wielaard <mark@klomp.org>
+   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/>.  */
+
+typedef struct
+  {
+    int q;
+    int r;
+  } div_t;
+
+typedef struct
+  {
+    long q;
+    long r;
+  } ldiv_t;
+
+typedef struct
+  {
+    float x;
+    float y;
+  } point_t;
+
+typedef struct
+  {
+    double x;
+    double y;
+  } dpoint_t;
+
+div_t __attribute__((__noinline__))
+div (int n, int d)
+{
+  div_t r;
+  r.q = n / d;
+  r.r = n % d;
+  return r;
+}
+
+ldiv_t __attribute__((__noinline__))
+ldiv (long n, long d)
+{
+  ldiv_t r;
+  r.q = n / d;
+  r.r = n % d;
+  return r;
+}
+
+point_t __attribute__((__noinline__))
+mkpt (float x, float y)
+{
+  point_t r;
+  r.x = x;
+  r.y = y;
+  return r;
+}
+
+dpoint_t __attribute__((__noinline__))
+dmkpt (double x, double y)
+{
+  dpoint_t r;
+  r.x = x;
+  r.y = y;
+  return r;
+}
+
+int
+main (void)
+{
+  div_t d = div (3, 2);
+  ldiv_t ld = ldiv (3, 2);
+  point_t p = mkpt (3.0f, 1.0f);
+  dpoint_t dp = dmkpt (3.0d, 1.0d);
+
+  return d.q - (int) p.y + ld.q - (int) dp.y;
+}
diff --git a/tests/funcretval_test_struct_riscv.bz2 b/tests/funcretval_test_struct_riscv.bz2
new file mode 100755
index 0000000000000000000000000000000000000000..de3e5ba9de36babad9344bb32968ef5af6c81798
GIT binary patch
literal 3821
zcmV<J4ifP~T4*^jL0KkKSpd72rvMGI|NsC0|NsC0|NsC0|9}7g|M&m*fBygVe$W2r
z?#$o(@BiQo>q*Y~Y5)ty-M6eX>VdmzUFvx+cD>JOwrQ>7RnXBRKnT(?HBZo|l<`j!
zCYYv1#XTd@9--=bo}<Xr^*v28c})#GN$E5_GLJ;snq>6@)OwFoL-j_Ck7`dT>NZo<
z18M-%)D1K=WIacy$n=0{f@)!@gwPSRXhAgeQ`Bj-4^Y!afCiZW4^RU@27mz3pa9XJ
z00E!?8UO=800w{nH8NxqNDW3wHuXvRh||(}Mw>`&OlcZu4@hb1XaS%A0ML4X(;xr<
z0002c00000G6$#zfD$4>69`klCPs}lO*g4OM4zQSN9v8FdY(y<`jgc3o~AV#4^2P=
zL7>s5fHVR6pwmMpfW&A4ri}mq27mz28a+Y)4K&ai8X64%&@=#O0iXah02%-Q41myR
z(9i$?000000004?8X5o^Y9djXrqd>-C#jks{ZXc#pa26v42=&+^#B7v13{n~05lB&
zpaV?+0000D02%-WfEQtVJ;s)h(~X-$1-}9t@tzkmP7^P*cHMHKR%t^dkX2eYHz)xY
zSMk(m6n0Qft0f{UC=3oNYmK@MZFUB>4Zb%LOL*wDM1+Y6p(fvx)an}`*qPLDdx5+w
zoakU9ofd{iFpNsLE(#?h0g6(wn4^waB!s08SH6g70Zt>5XyR^egPI+=c^-y-!RsW3
zjCtBGmQ`mYpms8hK}StOtMn)FYO5Xfyt8cVcPl!Q3vl*IlUj_A7Of^SO-q5(x(j`y
zXc@ZCff7wYp4#b>%9Z2I3X_XoRjgZ?7F!k?vhFD97`iHta)$&g{K9aQ*J=W4G&D96
z<K=VwA0v;pjA-unT#E5{Ts#@I7iYGfa@}Sbjku%{5is1j&i}#boypG)v2kZC^}pLI
zRpwi(uJJJ@m;$;%9KNLHlA4+X3W^E|To_6k)CdCr)-s(TQh_7U$%FcSE$<{F`GnZ1
z&Y|{fQivu5!h{$qO5LrXD)0=F<DNI4p`&mFkwgI!6S7?`5G1pW)5e@umOX{-fBNAN
z0Ja7QT1;A~2#KxYAjp@Qi#H1s0P=Oc&CV;py$9M-`mG8S9x~Kwq*54%Z+4&$F<{1h
z4JSz@kN_4Y2!QboBYnNHK>O?krQE^;kw0HgDx{2U?|SldE!eJg+%?!Ohq!+-$#IzD
zxK0$CihEnnVVP5KgMCsP(d4L;SRN9TG42+Xf+OTe^gJJ%gM;}vS=aE_%rtDjf!9W8
znnM^im^&XYvLy@?1Q!}A7?{L_bSw}O(neDRmrgU(w1YTk5~C3evFflhTr<+Np_JDg
zC1WViudyJ8gj&!ZRiU&Cp=hHJSQRx1kpduy8)^Bw<QAs3%nuT7Q3bZdIAL2nGY<y9
z#mazHwBLARYJ2MCi9F^SNn;U(pbzlHa{yH+mb#`2S}_>hK@icvlk{ih=T~B7WgNMr
z!>ShX$i>AP4Q3^MgLm3M8%-n%TbM#euC#b%9ZLj-Qvp753^-vKtSVyqRO{p@&|(cI
z6pN>yHcHCUz%nr+ItYMEs%f<hLem-&w<R)!K(9~@I-*(dT+9HsEWJoVEodQ{j*|jW
z5F>XGyFR80n$hAq`$|NSR7)I|#2c343z1`&$%xK{2WtThE;q<<5jN1*T2WzsVgQei
zC0H|;wq6Pa5<)buW7eihvWh&aya*eXmtFu^M1zRRxvkVN%Bk6_J$P2b=?s=+OIjEY
zn`Z^bC`GRffOZwsoN5PDB(vGgg2qrZgE>1(F<g$|WObD6O9x*P#i5C5(sb%?Q)5?P
zy;eF)OUM}Kie<G(fs$lH3W^7<wrsq@g2*0h7(tCQe^uF_$|HS&S-`5cs$fH2^1tq}
zLf8K>WOhY+T&gAn;e#_UX1E|`D;PmCY1E*E{MM%Gi+77=)o#kg#kHR25;U_vkj89r
zKe+l&g_qwo?s5b2kHJf6K+^?CHOf&SBP4>1WH4KHgiA~qZh;M?_1Y?ZA4)TQ<g52Q
zRs*ZsHF7e7fDccz80#=P4L{8dQLR~8s0t7pp&&afK-EpM9qzFYP&7k+RXs{D*sCZH
zD#_uJ5t9hEeAnG6N>392LX8ft_S=Isvy(3iFBz8{dW?ub0=UShjHH7AQ3E0%Bb^z5
zD9^PES+Rg0lztVYcI--jHpHw{$KAp@jCe_~59z8l<BZE2Ol+kgV+?P<Yo(YEOMAzm
zLx{m;ptW;t7geZj4cVaJ%X^n?RwY|jq97V*sWMQ(V>GZduC+XhNQEp|>|K)#_K<7{
zNHl}wb<86G7)6tjfWh%;^CU(L<QEy!i$wRqVTtaio@H!}9ifPwtOPa~&myee6$|P(
zawDwWa!*0N^_B6zuJdKge<+rT{t^FiR!z!Q?1++;IMJ9!+jPW35thOz1X&A#Ru3J;
zVx-}O@o0Rh3I5l~omw!J(hJlbNmy{ZO?B5kw#%BgGX`rl+8n_LTZ3ac=G-(IV%xh#
z@?SgkB8<^X#>oTG4@_*sV#>w!_zDzdCxH`baFn_+G+hFPi;0L2QFzsju%iZt6kW7V
zc?hVH#Exbes+^KBrYJTh@l1F4uZXLXOrl;i5cieDy0xabB7>ucc&aAzfWs9{Qg3?R
zze43nVeD_pMpH)5ZyH@6J0j?sSufHs53*z4kZml6m)LxL#2nfT?fWHGhPm!)63Nnx
z9b6@*@C~#!4T#)7edA!`W_KMz;@qutnS>pmOlWWR535}<-1N+f2v;9*k;AVgmKDQB
zhObL?QDJE<n@i+e&5qV(k&YTD6u`AWHUR~<MP-<vjzLvT9yDocsj|~VDbykkx(~0S
zJ!JeY!B^QJOoPFdjvczLS$*l5)u}%jM3qWII#kX6>OLMAZ7f!EK8}+{p~m03g`Y%Z
zYVx+dRod;{Wp)&&Q$|e#`u!x*V;3Y?h%B@OwV(?1xL#;3_!eg{GVehG?qWD|Ms_d2
z0M`dtxblryiI}R5#65nJZw}4|#k_uhFVoqfU04;BY?lc1NC?A$1w&&qh+{8_$>@F>
z+aCqJdrtW<#thuY?P|ar3~PWi8P<!dr<fx4I9`7%CB>ThIUH@l{s}697VG{5+wIzs
zI2jR!*bB9?2fN&CQ)9<X%Wi!2`^#bGRUsy&<*0Jdy3WgfxXF0elX7jyor^^PstIVA
z6a%)ffzb%qfXM?u!h$#7sTl_;>Z`lqPD>=|^#Ii2q)4Cy>CI`<?ZGZfvlQ=v@rL0H
zRc@Un2ozNHh+rl|;Lx*&;D_QsIfxNx;y|O335iH83Xc>Qy{l#K*zUoat_++Lj==%W
zCKRL<;AKi()vijDA+?5G(IU74ft`m;`7<fzwWD+|=yy~z1ObW;L8CB%8c*t~N*T`|
zL7$PV$Ot}b-5+Ohd_8BlAz+OQ3ZS4B6`)j$!ab(>RhOZ%W(XWo6G9^s!)Err;Smd{
z!VCxwg|AGS;)nt~D*DFJzg<Fx(C-w6|IwsZjqBR16_RH92=%lw^78269k*vN9)qfL
zfM6Mb?bZTgvK}D+?|Ff)0O8cq-yni8A#A@(rHw`44FXhd5#d7Q0m4q_LIolP?K26T
z8!NUk7FxTRd6?QYYPKc~`<a$RpgTY!0Uodgm_4b*K2p`$H80pJ0t~bxi<FF1I;C(3
zlL!kTq1UMEYnySQuo)FCbu~nXV6p%iGl3Z<#$N`93>btkN(u(F3Sk5A+-452TZ@Y}
z4`LBwhF@hN1ju|jt9T{X?L}&?GBw8yu}0)j($obNNuwAyHpdAn?Z~Ew!H^XkWI$Wy
z6bKV2^hF1N82GtZ(O^>D5a_BbM-SM&6`y6QWifIv-MgR!b2VASXve%f#MT*BQ@$G0
zhQM-VK|SLjKN?+wq`jUyCPAEfggFGtQAH1SKNNrli>Tx$0;&ZCSgC6S0MEagC|ove
zg*a}Z6a<mX0Ma>va9SYIe2`yUq_@m46U2C6@*0AP;zpA}Wf!px4JD^0JI=ol&VmdM
z(yw7uS_9l6+O7vq;|69X97vwWc>s0ez<EW4F`)-273|t7W>oMUtOVEr;SvOJrvC$q
z7Ro{>%Gm`8Nu&@sZJy2zoEM)L>k5P^jBB$#TTP)MmLwePh!-u`s1}rCwxArjSXQ4s
zjOdMV??fVPTDJ+7$tVe~nPgwEGbUp2OH-1o9oke4wnl8DNU)07F@m84;sUJ4(pLl^
z=LH2(kzhU+L1$Q-bSdiSw3XW_+z3JoJT_ned_`hcrbV47yfiw*4l<Z@-APS5LI@nd
zPk6vj3d(DX(S1F%qA_a1D#1_~F);-OLjw;i)>n8ib)pKk1?T9j7^fW)q#qK}0cQCF
z6Q<+J3k->isETJTCu&By5~W5Bq+(FU0Fi>GMu0z(Fw%|GkUESQvP|I1ELjZ}jbOth
z;uT0Yds@Poh%t+%VI}t$uAagrhKdUnA*7ax2BOfy6v}}1pIB))AEaprs3ig&FlBLe
z0LPLHa=&OW9^SLR@e~IMU}vyM4a#SxSSbQ!I(*TepI576Eq?bs-1(eS3U(ttBUj@h
z6^AMuKqBvKbFm!Fz8*HiRJi3j9Cf3DHt}=x%B6YlgON$*t+sDBk-KkD;=qR<btUw*
zHuhEh#1+g?X{KHd8bS`d#Qbj*Qu*BNzjtfqRMvlZ&qvX7;+EL(rMpSfj}D|@+aPFd
z5Se~Yo9tW{Y6;$nKiULBn7(=~PnqA3)z3!7KEy=|s$Pc<Tt%w{{F{n$xwc4%XeU*A
zVWjG?_1cJzQ#4JxklKnMa^z#G`gQyEC3%U=1tuB|24vRHYS)7XwtY)qTmnF0H&Ryc
z5tPB_W_#y($ROPsg&_zwtXNSdkuaS)>F?3e$l-7_5*scZG;BAf&q4znz4a`%T|}`#
zKg=@Gf)2R<Fn$N6OC*SGvN@(lT;Up(K;`zkUC%4b?(H%8Lf!z(%z*!l1Uim6kt#aW
j4jyw!ROBF#d#lbD#kh+7zY~*b|BJaIoG3^DUCYyehRwEF

literal 0
HcmV?d00001

diff --git a/tests/run-funcretval-struct-native.sh b/tests/run-funcretval-struct-native.sh
new file mode 100755
index 000000000000..798edb3b61b3
--- /dev/null
+++ b/tests/run-funcretval-struct-native.sh
@@ -0,0 +1,22 @@
+#! /bin/sh
+# Copyright (C) 2024 Mark J. Wielaard <mark@klomp.org>
+# 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
+
+# Just run it, we don't know what the native representation is.
+# But it should at least work and not error out.
+testrun $abs_builddir/funcretval -e $abs_builddir/funcretval_test_struct
diff --git a/tests/run-funcretval-struct.sh b/tests/run-funcretval-struct.sh
new file mode 100755
index 000000000000..92b2a3abca39
--- /dev/null
+++ b/tests/run-funcretval-struct.sh
@@ -0,0 +1,35 @@
+#! /bin/sh
+# Copyright (C) 2024 Mark J. Wielaard <mark@klomp.org>
+# 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
+
+# The test files are the native funcretval_test_struct files
+# funcretval_test_struct.c
+# See also run-funcretval-struct-native.sh
+
+testfiles funcretval_test_struct_riscv
+
+testrun_compare ${abs_top_builddir}/tests/funcretval \
+	-e funcretval_test_struct_riscv <<\EOF
+() main: return value location: {0x5a, 0}
+() dmkpt: return value location: {0x90, 0x2a} {0x93, 0x8} {0x90, 0x2b} {0x93, 0x8}
+() mkpt: return value location: {0x90, 0x2a}
+() ldiv: return value location: {0x5a, 0} {0x93, 0x8} {0x5b, 0} {0x93, 0x8}
+() div: return value location: {0x5a, 0}
+EOF
+
+exit 0
-- 
2.44.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] riscv: Partial implementation of flatten_aggregate
  2024-03-20 15:03 [PATCH] riscv: Partial implementation of flatten_aggregate Mark Wielaard
@ 2024-03-20 18:14 ` Aaron Merey
  2024-03-20 19:35   ` Mark Wielaard
  2024-03-20 20:17 ` Palmer Dabbelt
  1 sibling, 1 reply; 6+ messages in thread
From: Aaron Merey @ 2024-03-20 18:14 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: elfutils-devel

Hi Mark,

On Wed, Mar 20, 2024 at 11:03 AM Mark Wielaard <mark@klomp.org> wrote:
>
> dwfl_module_return_value_location would fail on riscv for functions
> which return a (small) struct. This patch implements the simplest
> cases of flatten_aggregate in backends/riscv_retval.c. It just handles
> structs containing one or two members of the same base type which fit
> completely or in pieces in one or two general or floating point
> registers.
>
> It also adds a specific test case run-funcretval-struct.sh containing
> small structs of ints, longs, floats and doubles. All these testscases
> now work for riscv. There is already a slightly more extensive
> testcase for this in tests/run-funcretval.sh but that only has a
> testcase for aarch64.

This patch LGTM.  I'm not able to test it myself on riscv but I see that
for the first time a build has successfully passed on the elfutils riscv
trybot [1].

Aaron

[1] https://builder.sourceware.org/buildbot/#/builders/273


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] riscv: Partial implementation of flatten_aggregate
  2024-03-20 18:14 ` Aaron Merey
@ 2024-03-20 19:35   ` Mark Wielaard
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Wielaard @ 2024-03-20 19:35 UTC (permalink / raw)
  To: Aaron Merey; +Cc: elfutils-devel, david.abdurachmanov

Hi Aaron,

On Wed, Mar 20, 2024 at 02:14:18PM -0400, Aaron Merey wrote:
> On Wed, Mar 20, 2024 at 11:03 AM Mark Wielaard <mark@klomp.org> wrote:
> >
> > dwfl_module_return_value_location would fail on riscv for functions
> > which return a (small) struct. This patch implements the simplest
> > cases of flatten_aggregate in backends/riscv_retval.c. It just handles
> > structs containing one or two members of the same base type which fit
> > completely or in pieces in one or two general or floating point
> > registers.
> >
> > It also adds a specific test case run-funcretval-struct.sh containing
> > small structs of ints, longs, floats and doubles. All these testscases
> > now work for riscv. There is already a slightly more extensive
> > testcase for this in tests/run-funcretval.sh but that only has a
> > testcase for aarch64.
> 
> This patch LGTM.  I'm not able to test it myself on riscv

This is why there is the run-funcretval-struct.sh with riscv binary
testcase. So at least you can see it generates a DWARF expression for
the return values in a riscv ELF binary.

> but I see that
> for the first time a build has successfully passed on the elfutils riscv
> trybot [1].

Yes, nice.
David also tested it on the fedora riscv koji builder:
http://fedora.riscv.rocks/koji/taskinfo?taskID=1654120

============================================================================
Testsuite summary for elfutils 0.191
============================================================================
# TOTAL: 276
# PASS:  270
# SKIP:  6
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================

So I think it generally works.

Thanks,

Mark

> 
> [1] https://builder.sourceware.org/buildbot/#/builders/273
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] riscv: Partial implementation of flatten_aggregate
  2024-03-20 15:03 [PATCH] riscv: Partial implementation of flatten_aggregate Mark Wielaard
  2024-03-20 18:14 ` Aaron Merey
@ 2024-03-20 20:17 ` Palmer Dabbelt
  2024-03-20 23:15   ` Mark Wielaard
  1 sibling, 1 reply; 6+ messages in thread
From: Palmer Dabbelt @ 2024-03-20 20:17 UTC (permalink / raw)
  To: mark; +Cc: elfutils-devel, mark

On Wed, 20 Mar 2024 08:03:12 PDT (-0700), mark@klomp.org wrote:
> dwfl_module_return_value_location would fail on riscv for functions
> which return a (small) struct. This patch implements the simplest
> cases of flatten_aggregate in backends/riscv_retval.c. It just handles
> structs containing one or two members of the same base type which fit
> completely or in pieces in one or two general or floating point
> registers.
>
> It also adds a specific test case run-funcretval-struct.sh containing
> small structs of ints, longs, floats and doubles. All these testscases
> now work for riscv. There is already a slightly more extensive
> testcase for this in tests/run-funcretval.sh but that only has a
> testcase for aarch64.
>
> 	 * backends/riscv_retval.c (flatten_aggregate_arg): Implement
>          for the simple cases where we have a struct with one or two
>          members of the same base type.
> 	 (pass_by_flattened_arg): Likewise. Call either
> 	 pass_in_gpr_lp64 or pass_in_fpr_lp64d.
> 	 (riscv_return_value_location_lp64ifd): Call
> 	 flatten_aggregate_arg including size.
> 	 * tests/Makefile.am (TESTS): Add run-funcretval-struct.sh
> 	 and run-funcretval-struct-native.sh.
> 	 (check_PROGRAMS): Add funcretval_test_struct.
> 	 (funcretval_test_struct_SOURCES): New.
> 	 (EXTRA_DIST): Add run-funcretval-struct.sh,
> 	 funcretval_test_struct_riscv.bz2 and
> 	 run-funcretval-struct-native.sh.
> 	 * tests/funcretval_test_struct_riscv.bz2: New test binary.
> 	 * tests/run-funcretval-struct-native.sh: New test.
> 	 * tests/run-funcretval-struct.sh: Likewise.
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=31142
>
> Signed-off-by: Mark Wielaard <mark@klomp.org>
> ---
>  backends/riscv_retval.c                | 123 ++++++++++++++++++++++---
>  tests/Makefile.am                      |   7 ++
>  tests/funcretval_test_struct.c         |  86 +++++++++++++++++
>  tests/funcretval_test_struct_riscv.bz2 | Bin 0 -> 3821 bytes
>  tests/run-funcretval-struct-native.sh  |  22 +++++
>  tests/run-funcretval-struct.sh         |  35 +++++++
>  6 files changed, 262 insertions(+), 11 deletions(-)
>  create mode 100644 tests/funcretval_test_struct.c
>  create mode 100755 tests/funcretval_test_struct_riscv.bz2
>  create mode 100755 tests/run-funcretval-struct-native.sh
>  create mode 100755 tests/run-funcretval-struct.sh
>
> diff --git a/backends/riscv_retval.c b/backends/riscv_retval.c
> index 0a1e02f81cd2..50c451a4ba32 100644
> --- a/backends/riscv_retval.c
> +++ b/backends/riscv_retval.c
> @@ -1,6 +1,7 @@
>  /* Function return value location for Linux/RISC-V ABI.
>     Copyright (C) 2018 Sifive, Inc.
>     Copyright (C) 2013 Red Hat, Inc.
> +   Copyright (C) 2024 Mark J. Wielaard <mark@klomp.org>
>     This file is part of elfutils.
>
>     This file is free software; you can redistribute it and/or modify
> @@ -105,23 +106,123 @@ pass_in_fpr_lp64d (const Dwarf_Op **locp, Dwarf_Word size)
>    return size <= 8 ? 1 : 4;
>  }
>
> +/* Checks if we can "flatten" the given type, Only handles the simple
> +   cases where we have a struct with one or two the same base type
> +   elements.  */
>  static int
> -flatten_aggregate_arg (Dwarf_Die *typedie __attribute__ ((unused)),
> -		       Dwarf_Die *arg0 __attribute__ ((unused)),
> -		       Dwarf_Die *arg1 __attribute__ ((unused)))
> +flatten_aggregate_arg (Dwarf_Die *typedie,
> +		       Dwarf_Word size,
> +		       Dwarf_Die *arg0,
> +		       Dwarf_Die *arg1)
>  {
> -  /* ??? */
> +  int tag0, tag1;
> +  Dwarf_Die member;
> +  Dwarf_Word encoding0, encoding1;
> +  Dwarf_Attribute attr;
> +  Dwarf_Word size0, size1;
> +
> +  if (size < 8 || size > 16)
> +    return 0;

IIUC elfutils only supports riscv64?  Assuming that's the case this 
looks fine.

> +
> +  if (dwarf_child (typedie, arg0) != 0)
> +    return 0;
> +
> +  tag0 = dwarf_tag (arg0);
> +  while (tag0 != -1 && tag0 != DW_TAG_member)
> +    {
> +      if (dwarf_siblingof (arg0, arg0) != 0)
> +	return 0;
> +      tag0 = dwarf_tag (arg0);
> +    }
> +
> +  if (tag0 != DW_TAG_member)
> +    return 0;
> +
> +  /* Remember where we are.  */
> +  member = *arg0;
> +
> +  tag0 = dwarf_peeled_die_type (arg0, arg0);
> +  if (tag0 != DW_TAG_base_type)
> +    return 0;
> +
> +  if (dwarf_attr_integrate (arg0, DW_AT_encoding, &attr) == NULL
> +      || dwarf_formudata (&attr, &encoding0) != 0)
> +    return 0;
> +
> +  if (dwarf_bytesize_aux (arg0, &size0) != 0)
> +    return 0;
> +
> +  if (size == size0)
> +    return 1; /* This one member is the whole size. */
> +
> +  if (size != 2 * size0)
> +    return 0; /* We only handle two of the same.  */
> +
> +  /* Look for another member with the same encoding.  */
> +  if (dwarf_siblingof (&member, arg1) != 0)
> +    return 0;
> +
> +  tag1 = dwarf_tag (arg1);
> +  while (tag1 != -1 && tag1 != DW_TAG_member)
> +    {
> +      if (dwarf_siblingof (arg1, arg1) != 0)
> +	return 0;
> +      tag1 = dwarf_tag (arg1);
> +    }
> +
> +  if (tag1 != DW_TAG_member)
> +    return 0;
> +
> +  tag1 = dwarf_peeled_die_type (arg1, arg1);
> +  if (tag1 != DW_TAG_base_type)
> +    return 0; /* We can only handle two equal base types for now. */
> +
> +  if (dwarf_attr_integrate (arg1, DW_AT_encoding, &attr) == NULL
> +      || dwarf_formudata (&attr, &encoding1) != 0
> +      || encoding0 != encoding1)
> +    return 0; /* We can only handle two of the same for now. */

We have that special case in the psABI where "struct { int; float; }" 
gets split into int/FP registers.  I don't really understand elfutils, 
but I think it'll be possible to trip this up with something along those 
lines.

> +
> +  if (dwarf_bytesize_aux (arg1, &size1) != 0)
> +    return 0;
> +
> +  if (size0 != size1)
> +    return 0; /* We can only handle two of the same for now. */
> +
>    return 1;
>  }
>
> +/* arg0 and arg1 should be the peeled die types found by
> +   flatten_aggregate_arg.  */
>  static int
> -pass_by_flattened_arg (const Dwarf_Op **locp __attribute__ ((unused)),
> -		       Dwarf_Word size __attribute__ ((unused)),
> -		       Dwarf_Die *arg0 __attribute__ ((unused)),
> -		       Dwarf_Die *arg1 __attribute__ ((unused)))
> +pass_by_flattened_arg (const Dwarf_Op **locp,
> +		       Dwarf_Word size,
> +		       Dwarf_Die *arg0,
> +		       Dwarf_Die *arg1 __attribute__((unused)))
>  {
> -  /* ??? */
> -  return -2;
> +  /* For now we just assume arg0 and arg1 are the same type and
> +     encoding.  */
> +  Dwarf_Word encoding;
> +  Dwarf_Attribute attr;
> +
> +  if (dwarf_attr_integrate (arg0, DW_AT_encoding, &attr) == NULL
> +      || dwarf_formudata (&attr, &encoding) != 0)
> +    return -1;
> +
> +  switch (encoding)
> +    {
> +    case DW_ATE_boolean:
> +    case DW_ATE_signed:
> +    case DW_ATE_unsigned:
> +    case DW_ATE_unsigned_char:
> +    case DW_ATE_signed_char:
> +      return pass_in_gpr_lp64 (locp, size);
> +
> +    case DW_ATE_float:
> +      return pass_in_fpr_lp64d (locp, size);
> +
> +    default:
> +      return -1;
> +    }
>  }
>
>  int
> @@ -158,7 +259,7 @@ riscv_return_value_location_lp64ifd (int fp, Dwarf_Die *functypedie,
>  	 provided the floating-point real is no more than FLEN bits wide and
>  	 the integer is no more than XLEN bits wide.  */
>        if (tag == DW_TAG_structure_type
> -	  && flatten_aggregate_arg (&typedie, &arg0, &arg1))
> +	  && flatten_aggregate_arg (&typedie, size, &arg0, &arg1))
>  	return pass_by_flattened_arg (locp, size, &arg0, &arg1);
>        /* Aggregates larger than 2*XLEN bits are passed by reference.  */
>        else if (size > 16)
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 9141074fe44c..9315ec3bbe4c 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -162,6 +162,7 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh newfile test-nlist \
>  	run-addr2line-C-test.sh \
>  	run-addr2line-i-test.sh run-addr2line-i-lex-test.sh \
>  	run-addr2line-i-demangle-test.sh run-addr2line-alt-debugpath.sh \
> +	run-funcretval-struct.sh \
>  	run-varlocs.sh run-exprlocs.sh run-varlocs-vars.sh run-funcretval.sh \
>  	run-backtrace-native.sh run-backtrace-data.sh run-backtrace-dwarf.sh \
>  	run-backtrace-native-biarch.sh run-backtrace-native-core.sh \
> @@ -284,6 +285,10 @@ funcretval_test__11_SOURCES = funcretval_test++11.cxx
>  TESTS += run-funcretval++11.sh
>  endif
>
> +check_PROGRAMS += funcretval_test_struct
> +funcretval_test_struct_SOURCES = funcretval_test_struct.c
> +TESTS += run-funcretval-struct-native.sh
> +
>  EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
>  	     run-ar-N.sh \
>  	     run-show-die-info.sh run-get-files.sh run-get-lines.sh \
> @@ -478,6 +483,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
>  	     testfile_aarch64_core.bz2 testfile_i686_core.bz2 \
>  	     addrx_constx-4.dwo.bz2 addrx_constx-5.dwo.bz2 \
>  	     testfile-addrx_constx-4.bz2 testfile-addrx_constx-5.bz2 \
> +	     run-funcretval-struct.sh funcretval_test_struct_riscv.bz2 \
>  	     run-funcretval.sh funcretval_test.c funcretval_test_aarch64.bz2 \
>  	     run-backtrace-data.sh run-backtrace-dwarf.sh cleanup-13.c \
>  	     run-backtrace-native.sh run-backtrace-native-biarch.sh \
> @@ -635,6 +641,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
>  	     testfile_nvidia_linemap.bz2 \
>  	     testfile-largealign.o.bz2 run-strip-largealign.sh \
>  	     run-funcretval++11.sh \
> +	     run-funcretval-struct-native.sh \
>  	     test-ar-duplicates.a.bz2 \
>  	     run-dwfl-core-noncontig.sh testcore-noncontig.bz2 \
>  	     testfile-dwarf5-line-clang.bz2 \
> diff --git a/tests/funcretval_test_struct.c b/tests/funcretval_test_struct.c
> new file mode 100644
> index 000000000000..df94bde0a42d
> --- /dev/null
> +++ b/tests/funcretval_test_struct.c
> @@ -0,0 +1,86 @@
> +/* Copyright (C) 2024 Mark J. Wielaard <mark@klomp.org>
> +   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/>.  */
> +
> +typedef struct
> +  {
> +    int q;
> +    int r;
> +  } div_t;
> +
> +typedef struct
> +  {
> +    long q;
> +    long r;
> +  } ldiv_t;
> +
> +typedef struct
> +  {
> +    float x;
> +    float y;
> +  } point_t;
> +
> +typedef struct
> +  {
> +    double x;
> +    double y;
> +  } dpoint_t;
> +
> +div_t __attribute__((__noinline__))
> +div (int n, int d)
> +{
> +  div_t r;
> +  r.q = n / d;
> +  r.r = n % d;
> +  return r;
> +}
> +
> +ldiv_t __attribute__((__noinline__))
> +ldiv (long n, long d)
> +{
> +  ldiv_t r;
> +  r.q = n / d;
> +  r.r = n % d;
> +  return r;
> +}
> +
> +point_t __attribute__((__noinline__))
> +mkpt (float x, float y)
> +{
> +  point_t r;
> +  r.x = x;
> +  r.y = y;
> +  return r;
> +}
> +
> +dpoint_t __attribute__((__noinline__))
> +dmkpt (double x, double y)
> +{
> +  dpoint_t r;
> +  r.x = x;
> +  r.y = y;
> +  return r;
> +}
> +
> +int
> +main (void)
> +{
> +  div_t d = div (3, 2);
> +  ldiv_t ld = ldiv (3, 2);
> +  point_t p = mkpt (3.0f, 1.0f);
> +  dpoint_t dp = dmkpt (3.0d, 1.0d);
> +
> +  return d.q - (int) p.y + ld.q - (int) dp.y;
> +}
> diff --git a/tests/funcretval_test_struct_riscv.bz2 b/tests/funcretval_test_struct_riscv.bz2
> new file mode 100755
> index 0000000000000000000000000000000000000000..de3e5ba9de36babad9344bb32968ef5af6c81798
> GIT binary patch
> literal 3821
> zcmV<J4ifP~T4*^jL0KkKSpd72rvMGI|NsC0|NsC0|NsC0|9}7g|M&m*fBygVe$W2r
> z?#$o(@BiQo>q*Y~Y5)ty-M6eX>VdmzUFvx+cD>JOwrQ>7RnXBRKnT(?HBZo|l<`j!
> zCYYv1#XTd@9--=bo}<Xr^*v28c})#GN$E5_GLJ;snq>6@)OwFoL-j_Ck7`dT>NZo<
> z18M-%)D1K=WIacy$n=0{f@)!@gwPSRXhAgeQ`Bj-4^Y!afCiZW4^RU@27mz3pa9XJ
> z00E!?8UO=800w{nH8NxqNDW3wHuXvRh||(}Mw>`&OlcZu4@hb1XaS%A0ML4X(;xr<
> z0002c00000G6$#zfD$4>69`klCPs}lO*g4OM4zQSN9v8FdY(y<`jgc3o~AV#4^2P=
> zL7>s5fHVR6pwmMpfW&A4ri}mq27mz28a+Y)4K&ai8X64%&@=#O0iXah02%-Q41myR
> z(9i$?000000004?8X5o^Y9djXrqd>-C#jks{ZXc#pa26v42=&+^#B7v13{n~05lB&
> zpaV?+0000D02%-WfEQtVJ;s)h(~X-$1-}9t@tzkmP7^P*cHMHKR%t^dkX2eYHz)xY
> zSMk(m6n0Qft0f{UC=3oNYmK@MZFUB>4Zb%LOL*wDM1+Y6p(fvx)an}`*qPLDdx5+w
> zoakU9ofd{iFpNsLE(#?h0g6(wn4^waB!s08SH6g70Zt>5XyR^egPI+=c^-y-!RsW3
> zjCtBGmQ`mYpms8hK}StOtMn)FYO5Xfyt8cVcPl!Q3vl*IlUj_A7Of^SO-q5(x(j`y
> zXc@ZCff7wYp4#b>%9Z2I3X_XoRjgZ?7F!k?vhFD97`iHta)$&g{K9aQ*J=W4G&D96
> z<K=VwA0v;pjA-unT#E5{Ts#@I7iYGfa@}Sbjku%{5is1j&i}#boypG)v2kZC^}pLI
> zRpwi(uJJJ@m;$;%9KNLHlA4+X3W^E|To_6k)CdCr)-s(TQh_7U$%FcSE$<{F`GnZ1
> z&Y|{fQivu5!h{$qO5LrXD)0=F<DNI4p`&mFkwgI!6S7?`5G1pW)5e@umOX{-fBNAN
> z0Ja7QT1;A~2#KxYAjp@Qi#H1s0P=Oc&CV;py$9M-`mG8S9x~Kwq*54%Z+4&$F<{1h
> z4JSz@kN_4Y2!QboBYnNHK>O?krQE^;kw0HgDx{2U?|SldE!eJg+%?!Ohq!+-$#IzD
> zxK0$CihEnnVVP5KgMCsP(d4L;SRN9TG42+Xf+OTe^gJJ%gM;}vS=aE_%rtDjf!9W8
> znnM^im^&XYvLy@?1Q!}A7?{L_bSw}O(neDRmrgU(w1YTk5~C3evFflhTr<+Np_JDg
> zC1WViudyJ8gj&!ZRiU&Cp=hHJSQRx1kpduy8)^Bw<QAs3%nuT7Q3bZdIAL2nGY<y9
> z#mazHwBLARYJ2MCi9F^SNn;U(pbzlHa{yH+mb#`2S}_>hK@icvlk{ih=T~B7WgNMr
> z!>ShX$i>AP4Q3^MgLm3M8%-n%TbM#euC#b%9ZLj-Qvp753^-vKtSVyqRO{p@&|(cI
> z6pN>yHcHCUz%nr+ItYMEs%f<hLem-&w<R)!K(9~@I-*(dT+9HsEWJoVEodQ{j*|jW
> z5F>XGyFR80n$hAq`$|NSR7)I|#2c343z1`&$%xK{2WtThE;q<<5jN1*T2WzsVgQei
> zC0H|;wq6Pa5<)buW7eihvWh&aya*eXmtFu^M1zRRxvkVN%Bk6_J$P2b=?s=+OIjEY
> zn`Z^bC`GRffOZwsoN5PDB(vGgg2qrZgE>1(F<g$|WObD6O9x*P#i5C5(sb%?Q)5?P
> zy;eF)OUM}Kie<G(fs$lH3W^7<wrsq@g2*0h7(tCQe^uF_$|HS&S-`5cs$fH2^1tq}
> zLf8K>WOhY+T&gAn;e#_UX1E|`D;PmCY1E*E{MM%Gi+77=)o#kg#kHR25;U_vkj89r
> zKe+l&g_qwo?s5b2kHJf6K+^?CHOf&SBP4>1WH4KHgiA~qZh;M?_1Y?ZA4)TQ<g52Q
> zRs*ZsHF7e7fDccz80#=P4L{8dQLR~8s0t7pp&&afK-EpM9qzFYP&7k+RXs{D*sCZH
> zD#_uJ5t9hEeAnG6N>392LX8ft_S=Isvy(3iFBz8{dW?ub0=UShjHH7AQ3E0%Bb^z5
> zD9^PES+Rg0lztVYcI--jHpHw{$KAp@jCe_~59z8l<BZE2Ol+kgV+?P<Yo(YEOMAzm
> zLx{m;ptW;t7geZj4cVaJ%X^n?RwY|jq97V*sWMQ(V>GZduC+XhNQEp|>|K)#_K<7{
> zNHl}wb<86G7)6tjfWh%;^CU(L<QEy!i$wRqVTtaio@H!}9ifPwtOPa~&myee6$|P(
> zawDwWa!*0N^_B6zuJdKge<+rT{t^FiR!z!Q?1++;IMJ9!+jPW35thOz1X&A#Ru3J;
> zVx-}O@o0Rh3I5l~omw!J(hJlbNmy{ZO?B5kw#%BgGX`rl+8n_LTZ3ac=G-(IV%xh#
> z@?SgkB8<^X#>oTG4@_*sV#>w!_zDzdCxH`baFn_+G+hFPi;0L2QFzsju%iZt6kW7V
> zc?hVH#Exbes+^KBrYJTh@l1F4uZXLXOrl;i5cieDy0xabB7>ucc&aAzfWs9{Qg3?R
> zze43nVeD_pMpH)5ZyH@6J0j?sSufHs53*z4kZml6m)LxL#2nfT?fWHGhPm!)63Nnx
> z9b6@*@C~#!4T#)7edA!`W_KMz;@qutnS>pmOlWWR535}<-1N+f2v;9*k;AVgmKDQB
> zhObL?QDJE<n@i+e&5qV(k&YTD6u`AWHUR~<MP-<vjzLvT9yDocsj|~VDbykkx(~0S
> zJ!JeY!B^QJOoPFdjvczLS$*l5)u}%jM3qWII#kX6>OLMAZ7f!EK8}+{p~m03g`Y%Z
> zYVx+dRod;{Wp)&&Q$|e#`u!x*V;3Y?h%B@OwV(?1xL#;3_!eg{GVehG?qWD|Ms_d2
> z0M`dtxblryiI}R5#65nJZw}4|#k_uhFVoqfU04;BY?lc1NC?A$1w&&qh+{8_$>@F>
> z+aCqJdrtW<#thuY?P|ar3~PWi8P<!dr<fx4I9`7%CB>ThIUH@l{s}697VG{5+wIzs
> zI2jR!*bB9?2fN&CQ)9<X%Wi!2`^#bGRUsy&<*0Jdy3WgfxXF0elX7jyor^^PstIVA
> z6a%)ffzb%qfXM?u!h$#7sTl_;>Z`lqPD>=|^#Ii2q)4Cy>CI`<?ZGZfvlQ=v@rL0H
> zRc@Un2ozNHh+rl|;Lx*&;D_QsIfxNx;y|O335iH83Xc>Qy{l#K*zUoat_++Lj==%W
> zCKRL<;AKi()vijDA+?5G(IU74ft`m;`7<fzwWD+|=yy~z1ObW;L8CB%8c*t~N*T`|
> zL7$PV$Ot}b-5+Ohd_8BlAz+OQ3ZS4B6`)j$!ab(>RhOZ%W(XWo6G9^s!)Err;Smd{
> z!VCxwg|AGS;)nt~D*DFJzg<Fx(C-w6|IwsZjqBR16_RH92=%lw^78269k*vN9)qfL
> zfM6Mb?bZTgvK}D+?|Ff)0O8cq-yni8A#A@(rHw`44FXhd5#d7Q0m4q_LIolP?K26T
> z8!NUk7FxTRd6?QYYPKc~`<a$RpgTY!0Uodgm_4b*K2p`$H80pJ0t~bxi<FF1I;C(3
> zlL!kTq1UMEYnySQuo)FCbu~nXV6p%iGl3Z<#$N`93>btkN(u(F3Sk5A+-452TZ@Y}
> z4`LBwhF@hN1ju|jt9T{X?L}&?GBw8yu}0)j($obNNuwAyHpdAn?Z~Ew!H^XkWI$Wy
> z6bKV2^hF1N82GtZ(O^>D5a_BbM-SM&6`y6QWifIv-MgR!b2VASXve%f#MT*BQ@$G0
> zhQM-VK|SLjKN?+wq`jUyCPAEfggFGtQAH1SKNNrli>Tx$0;&ZCSgC6S0MEagC|ove
> zg*a}Z6a<mX0Ma>va9SYIe2`yUq_@m46U2C6@*0AP;zpA}Wf!px4JD^0JI=ol&VmdM
> z(yw7uS_9l6+O7vq;|69X97vwWc>s0ez<EW4F`)-273|t7W>oMUtOVEr;SvOJrvC$q
> z7Ro{>%Gm`8Nu&@sZJy2zoEM)L>k5P^jBB$#TTP)MmLwePh!-u`s1}rCwxArjSXQ4s
> zjOdMV??fVPTDJ+7$tVe~nPgwEGbUp2OH-1o9oke4wnl8DNU)07F@m84;sUJ4(pLl^
> z=LH2(kzhU+L1$Q-bSdiSw3XW_+z3JoJT_ned_`hcrbV47yfiw*4l<Z@-APS5LI@nd
> zPk6vj3d(DX(S1F%qA_a1D#1_~F);-OLjw;i)>n8ib)pKk1?T9j7^fW)q#qK}0cQCF
> z6Q<+J3k->isETJTCu&By5~W5Bq+(FU0Fi>GMu0z(Fw%|GkUESQvP|I1ELjZ}jbOth
> z;uT0Yds@Poh%t+%VI}t$uAagrhKdUnA*7ax2BOfy6v}}1pIB))AEaprs3ig&FlBLe
> z0LPLHa=&OW9^SLR@e~IMU}vyM4a#SxSSbQ!I(*TepI576Eq?bs-1(eS3U(ttBUj@h
> z6^AMuKqBvKbFm!Fz8*HiRJi3j9Cf3DHt}=x%B6YlgON$*t+sDBk-KkD;=qR<btUw*
> zHuhEh#1+g?X{KHd8bS`d#Qbj*Qu*BNzjtfqRMvlZ&qvX7;+EL(rMpSfj}D|@+aPFd
> z5Se~Yo9tW{Y6;$nKiULBn7(=~PnqA3)z3!7KEy=|s$Pc<Tt%w{{F{n$xwc4%XeU*A
> zVWjG?_1cJzQ#4JxklKnMa^z#G`gQyEC3%U=1tuB|24vRHYS)7XwtY)qTmnF0H&Ryc
> z5tPB_W_#y($ROPsg&_zwtXNSdkuaS)>F?3e$l-7_5*scZG;BAf&q4znz4a`%T|}`#
> zKg=@Gf)2R<Fn$N6OC*SGvN@(lT;Up(K;`zkUC%4b?(H%8Lf!z(%z*!l1Uim6kt#aW
> j4jyw!ROBF#d#lbD#kh+7zY~*b|BJaIoG3^DUCYyehRwEF
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/run-funcretval-struct-native.sh b/tests/run-funcretval-struct-native.sh
> new file mode 100755
> index 000000000000..798edb3b61b3
> --- /dev/null
> +++ b/tests/run-funcretval-struct-native.sh
> @@ -0,0 +1,22 @@
> +#! /bin/sh
> +# Copyright (C) 2024 Mark J. Wielaard <mark@klomp.org>
> +# 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
> +
> +# Just run it, we don't know what the native representation is.
> +# But it should at least work and not error out.
> +testrun $abs_builddir/funcretval -e $abs_builddir/funcretval_test_struct
> diff --git a/tests/run-funcretval-struct.sh b/tests/run-funcretval-struct.sh
> new file mode 100755
> index 000000000000..92b2a3abca39
> --- /dev/null
> +++ b/tests/run-funcretval-struct.sh
> @@ -0,0 +1,35 @@
> +#! /bin/sh
> +# Copyright (C) 2024 Mark J. Wielaard <mark@klomp.org>
> +# 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
> +
> +# The test files are the native funcretval_test_struct files
> +# funcretval_test_struct.c
> +# See also run-funcretval-struct-native.sh
> +
> +testfiles funcretval_test_struct_riscv
> +
> +testrun_compare ${abs_top_builddir}/tests/funcretval \
> +	-e funcretval_test_struct_riscv <<\EOF
> +() main: return value location: {0x5a, 0}
> +() dmkpt: return value location: {0x90, 0x2a} {0x93, 0x8} {0x90, 0x2b} {0x93, 0x8}
> +() mkpt: return value location: {0x90, 0x2a}
> +() ldiv: return value location: {0x5a, 0} {0x93, 0x8} {0x5b, 0} {0x93, 0x8}
> +() div: return value location: {0x5a, 0}
> +EOF
> +
> +exit 0

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>

though I'm not sure that means a whole lot, as I haven't really poked 
around elfutils before.

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] riscv: Partial implementation of flatten_aggregate
  2024-03-20 20:17 ` Palmer Dabbelt
@ 2024-03-20 23:15   ` Mark Wielaard
  2024-03-20 23:30     ` Palmer Dabbelt
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Wielaard @ 2024-03-20 23:15 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: elfutils-devel

Hi Palmer,

On Wed, Mar 20, 2024 at 01:17:14PM -0700, Palmer Dabbelt wrote:
> >+flatten_aggregate_arg (Dwarf_Die *typedie,
> >+		       Dwarf_Word size,
> >+		       Dwarf_Die *arg0,
> >+		       Dwarf_Die *arg1)
> > {
> >-  /* ??? */
> >+  int tag0, tag1;
> >+  Dwarf_Die member;
> >+  Dwarf_Word encoding0, encoding1;
> >+  Dwarf_Attribute attr;
> >+  Dwarf_Word size0, size1;
> >+
> >+  if (size < 8 || size > 16)
> >+    return 0;
> 
> IIUC elfutils only supports riscv64?  Assuming that's the case this
> looks fine.

Yes, at the moment we only support riscv64, we do accept 32bit riscv
ELF files, but don't know anything about how it handles calling
conventions, core file layout, etc.

> >+  tag1 = dwarf_peeled_die_type (arg1, arg1);
> >+  if (tag1 != DW_TAG_base_type)
> >+    return 0; /* We can only handle two equal base types for now. */
> >+
> >+  if (dwarf_attr_integrate (arg1, DW_AT_encoding, &attr) == NULL
> >+      || dwarf_formudata (&attr, &encoding1) != 0
> >+      || encoding0 != encoding1)
> >+    return 0; /* We can only handle two of the same for now. */
> 
> We have that special case in the psABI where "struct { int; float;
> }" gets split into int/FP registers.  I don't really understand
> elfutils, but I think it'll be possible to trip this up with
> something along those lines.

aha, I see...

"A struct containing one floating-point real and one integer (or
bitfield), in either order, is passed in a floating-point register and
an integer register, provided the floating-point real is no more than
ABI_FLEN bits wide and the integer is no more than XLEN bits wide."

Yes, that isn't currently recognized. I'll try to add an update to
handle this specific case.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] riscv: Partial implementation of flatten_aggregate
  2024-03-20 23:15   ` Mark Wielaard
@ 2024-03-20 23:30     ` Palmer Dabbelt
  0 siblings, 0 replies; 6+ messages in thread
From: Palmer Dabbelt @ 2024-03-20 23:30 UTC (permalink / raw)
  To: mark; +Cc: elfutils-devel

On Wed, 20 Mar 2024 16:15:30 PDT (-0700), mark@klomp.org wrote:
> Hi Palmer,
>
> On Wed, Mar 20, 2024 at 01:17:14PM -0700, Palmer Dabbelt wrote:
>> >+flatten_aggregate_arg (Dwarf_Die *typedie,
>> >+		       Dwarf_Word size,
>> >+		       Dwarf_Die *arg0,
>> >+		       Dwarf_Die *arg1)
>> > {
>> >-  /* ??? */
>> >+  int tag0, tag1;
>> >+  Dwarf_Die member;
>> >+  Dwarf_Word encoding0, encoding1;
>> >+  Dwarf_Attribute attr;
>> >+  Dwarf_Word size0, size1;
>> >+
>> >+  if (size < 8 || size > 16)
>> >+    return 0;
>>
>> IIUC elfutils only supports riscv64?  Assuming that's the case this
>> looks fine.
>
> Yes, at the moment we only support riscv64, we do accept 32bit riscv
> ELF files, but don't know anything about how it handles calling
> conventions, core file layout, etc.
>
>> >+  tag1 = dwarf_peeled_die_type (arg1, arg1);
>> >+  if (tag1 != DW_TAG_base_type)
>> >+    return 0; /* We can only handle two equal base types for now. */
>> >+
>> >+  if (dwarf_attr_integrate (arg1, DW_AT_encoding, &attr) == NULL
>> >+      || dwarf_formudata (&attr, &encoding1) != 0
>> >+      || encoding0 != encoding1)
>> >+    return 0; /* We can only handle two of the same for now. */
>>
>> We have that special case in the psABI where "struct { int; float;
>> }" gets split into int/FP registers.  I don't really understand
>> elfutils, but I think it'll be possible to trip this up with
>> something along those lines.
>
> aha, I see...
>
> "A struct containing one floating-point real and one integer (or
> bitfield), in either order, is passed in a floating-point register and
> an integer register, provided the floating-point real is no more than
> ABI_FLEN bits wide and the integer is no more than XLEN bits wide."
>
> Yes, that isn't currently recognized. I'll try to add an update to
> handle this specific case.

Cool, thanks.  In general the RISC-V specs have a lot of these non-local 
statements, so it's kind of hard to just read sub-components of it as 
you're likely to get tripped up.

>
> Thanks,
>
> Mark

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-03-20 23:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-20 15:03 [PATCH] riscv: Partial implementation of flatten_aggregate Mark Wielaard
2024-03-20 18:14 ` Aaron Merey
2024-03-20 19:35   ` Mark Wielaard
2024-03-20 20:17 ` Palmer Dabbelt
2024-03-20 23:15   ` Mark Wielaard
2024-03-20 23:30     ` Palmer Dabbelt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).