[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Fixing multiple aliasing of a symbol



I have added test case for multiple aliasing of a symbol. Updated patch is 
available in the email attachment.

Thanks
Sinny Kumari

On Thursday 19 Jun 2014 3:11:10 PM Sinny Kumari wrote:
> Thank you so much for feedback. I have made changes according to instruction
> mentioned in the email and attached updated patch.
> 
> On Wednesday 18 Jun 2014 6:03:47 PM you wrote:
> > Hello Sinny,
> > 
> > Thank you very much for the patch, which looks good to me, overall.
> > 
> > I just have a few nits to point you to, if you don't mind.
> > 
> > First, you need to really locally commit the thing and sign your work,
> > and then use 'git format-patch' to get the patch.  This is explained in
> > the CONTRIBUTING file in the source tree.  Also, please read the
> > COMMIT-LOG-GUIDELINES file of the source tree to know how to format the
> > commit log.
> > 
> > You can look at existing commits in the history to see how they are
> > formatted, for instancve.
> > 
> > Now the nits of the patch itself.
> > 
> > diff --git a/src/abg-reader.cc b/src/abg-reader.cc
> > [...]
> > 
> > @@ -1650,13 +1651,26 @@ build_elf_symbol_db(read_context& ctxt,
> > [...]
> > 
> >      {
> >      
> >        if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(x->first, "alias"))
> >  	
> >  	{
> > 
> > -	  string alias_id = CHAR_STR(s);
> > -	  string_elf_symbol_sptr_map_type::const_iterator i =
> > -	    id_sym_map.find(alias_id);
> > -	  assert(i != id_sym_map.end());
> > -	  assert(i->second->is_main_symbol());
> > +      string alias_id = CHAR_STR(s);
> > 
> > -	  x->second->get_main_symbol()->add_alias(i->second.get());
> > +      // Symbol aliases can be multiple separtaed by comma(,), split them
> > 
> > In the comment, I guess you meant 'separated'.
> > 
> > +      std::vector<std::string> elems;
> > +      std::stringstream aliases(alias_id);
> > +      std::string item;
> > +      while (std::getline(aliases, item, ','))
> > +      {
> > +        elems.push_back(item);
> > +      }
> > 
> > I think we need a test case for this, similar to what is found in the
> > tests/data/test-read-dwarf/ directory.  Basically, there would be a short
> > program compiled in a *.so file, and that contains a symbol with several
> > aliases.  The tests/test-read-dwarf.cc harness would then read the .so
> > file, emit an XML for that and compare it to a reference XML.  If you
> > can send me the source code for the program that is to be compiled into
> > the *.so, I can add it to the test suite, or we can discuss how to do
> > that if you want.  I think I need to add documentation for all this ;-)
> 
> I will work on writing test case for same.
> 
> 
> Thanks
> SInny Kumari
>From 67fb8fb4cd78a4a904895a2cd2890466f5aafa23 Mon Sep 17 00:00:00 2001
From: Sinny Kumari <skumari@redhat.com>
Date: Thu, 19 Jun 2014 12:13:54 +0530
Subject: [PATCH 1/1] Keep symbol's multiple aliases within single attribute
 separated by comma(,)

    * src/abg-writer.cc (write_elf_symbol_aliases): Changing function
    to keep multiple symbol aliases within one alias attribute
    * src/abg-reader.cc (build_elf_symbol_db): Changing function to read
    symbol's alias attribute and split if multiple alias exist with comma(,)
    asi a delimiter and add all aliases to main symbol
    * tests/data/test-read-dwarf/test3.c: Test file to generate multiple aliases
    * tests/data/test-read-dwarf/test3.so: Test shared library having multiple
    aliases of a symbol
    * tests/data/test-read-dwarf/test3.so.abi: XML file containing dwarf
    information from test3.so
    * tests/test-read-dwarf.cc (in_out_specs): Add the new test above
    * tests/Makefile.am: Add tests/data/test-read-dwarf/test3.c,
    tests/data/test-read-dwarf/test3.so and tests/data/test-read-dwarf/test3.so.abi
    to the distribution

Signed-off-by: Sinny Kumari <skumari@redhat.com>
---
 src/abg-reader.cc                       |  26 +++++++++++++++++++-------
 src/abg-writer.cc                       |   7 ++++++-
 tests/Makefile.am                       |   2 ++
 tests/data/test-read-dwarf/test3.c      |  11 +++++++++++
 tests/data/test-read-dwarf/test3.so     | Bin 0 -> 8851 bytes
 tests/data/test-read-dwarf/test3.so.abi |  14 ++++++++++++++
 tests/test-read-dwarf.cc                |   5 +++++
 7 files changed, 57 insertions(+), 8 deletions(-)
 create mode 100644 tests/data/test-read-dwarf/test3.c
 create mode 100755 tests/data/test-read-dwarf/test3.so
 create mode 100644 tests/data/test-read-dwarf/test3.so.abi

diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 21ec927..1862816 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -29,6 +29,7 @@
 #include <tr1/unordered_map>
 #include <deque>
 #include <assert.h>
+#include <sstream>
 #include <libxml/xmlstring.h>
 #include <libxml/xmlreader.h>
 #include "abg-libxml-utils.h"
@@ -1650,13 +1651,24 @@ build_elf_symbol_db(read_context& ctxt,
     {
       if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(x->first, "alias"))
 	{
-	  string alias_id = CHAR_STR(s);
-	  string_elf_symbol_sptr_map_type::const_iterator i =
-	    id_sym_map.find(alias_id);
-	  assert(i != id_sym_map.end());
-	  assert(i->second->is_main_symbol());
-
-	  x->second->get_main_symbol()->add_alias(i->second.get());
+      string alias_id = CHAR_STR(s);
+
+      // Symbol aliases can be multiple separated by comma(,), split them
+      std::vector<std::string> elems;
+      std::stringstream aliases(alias_id);
+      std::string item;
+      while (std::getline(aliases, item, ','))
+        elems.push_back(item);
+      for (std::vector<string>::iterator alias = elems.begin();
+           alias != elems.end(); alias++)
+        {
+          string_elf_symbol_sptr_map_type::const_iterator i =
+          id_sym_map.find(*alias);
+          assert(i != id_sym_map.end());
+          assert(i->second->is_main_symbol());
+
+          x->second->get_main_symbol()->add_alias(i->second.get());
+        }
 	}
     }
 
diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index 07bb3a2..84b8e48 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -652,11 +652,16 @@ write_elf_symbol_aliases(const elf_symbol& sym, ostream& o)
     return false;
 
   bool emitted = false;
+  o << " alias='";
   for (elf_symbol* s = sym.get_next_alias();
        !s->is_main_symbol();
        s = s->get_next_alias())
     {
-      o << " alias='" << s->get_id_string() << "'";
+      if (s->get_next_alias() == s->get_main_symbol())
+          o << s->get_id_string() << "'";
+      else
+          o << s->get_id_string() << ",";
+
       emitted = true;
     }
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 30c63bf..677d3f1 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -173,6 +173,8 @@ tests/data/test-read-dwarf/test2-0.cc		\
 tests/data/test-read-dwarf/test2-1.cc		\
 tests/data/test-read-dwarf/test2.so		\
 tests/data/test-read-dwarf/test2.so.abi		\
+tests/data/test-read-dwarf/test3.c		\
+tests/data/test-read-dwarf/test3.so		\
 \
 data/test-diff-filter/test0-v0.cc		\
 data/test-diff-filter/test0-v1.cc		\
diff --git a/tests/data/test-read-dwarf/test3.c b/tests/data/test-read-dwarf/test3.c
new file mode 100644
index 0000000..2808db4
--- /dev/null
+++ b/tests/data/test-read-dwarf/test3.c
@@ -0,0 +1,11 @@
+// Test file for creating multiple alias for a symbol
+
+void __foo(void);
+void foo(void) __attribute__((weak, alias("__foo")));
+void foo__(void) __attribute__((weak, alias("__foo")));
+void __foo__(void) __attribute__((alias("__foo")));
+
+void __foo(void)
+{
+
+}
diff --git a/tests/data/test-read-dwarf/test3.so b/tests/data/test-read-dwarf/test3.so
new file mode 100755
index 0000000000000000000000000000000000000000..4c426fc3c202d2f2dd027ffd8825ccfea36b2a09
GIT binary patch
literal 8851
zcmeHMeQX>@6`#Aa<8$qt9j6VB(~?z48YjZ*k0i!QWMb!H`x5N5xHbh*YqfW~XJ30C
zW_K^PO8`p_At|z`5E4P+kAj3i2%#V$l=2~5j4C7$GKET^e??TKc8G-2isFi@=Dpc@
z=iRxz773|;c}}};<~Q$S_RZd#nSEax7#<9VLX1j;eVp;u4j1VY9nHImgRwr=!)oBJ
zu!lHr^;TC|?NW^k5oI8S1#O^-)t%Tvs`zMMm^WEMP?EzY%0**te>A4>3egolc*clE
zT{cz)6tCZjewiK=d_<8K%cHM32G-vZ{*dW|oQpA{B=<>>6C#4+y-B0!9u@W~`K4L-
zgQ$2M{%rSyjtE>s7slCNjaRO$z4^@A^Tx=YOB>H^y!}vh3!1=woD%Z?k6uzD^HqJd
zh}{PlPRE&nm+xM+uh#wI)vsSlU$Gl*eeTl_KEHS`R<-=YxjTRUXYBjy{}A2>=717m
zwd@DdOM(7T0(fy8LAYK4@2P-~0)7B4xhw+E$R1#=jf$^p23iML9a|NXM052V;Eik#
z>yY@rP=Ws&f?w8;^Vt;OhtG4o-0mKZ*RypR2(+;j;c5yPC)2daTp_R7j&3=c#<Wa6
z<1j4^glXr-Ml{2;%*l-HnAX@xJX^?{V|p@cx_!YtT548D1$s8~G%%jOsErB@@vLs!
zrVTdJg@Strli5r%rP>9xn`x$@J34eEZQC6YKzV~XrtKU<mQFS_eC|wK>r%Va6a2a8
z7KS%b{$U?t3qa|AQEbW7G8SexgdoKW`hVXLep?rDgz#zdC&dXN&qVuHgTfJHD}@@U
z-}d16pP{nq!Rfz_ed`|F`|59aa2g8oH;eZR|94*fiU;@dCl>BT6JM=vyH8<>FE2UO
zs~-Wu#KLc*%cyOuY@X->md!)Q;oh*n4{F$+LWZ^5n@|oN#Ww!!)>fdISgdXZAhFon
z0%SaKWFv9ywZy_kY$iOh3<iH%>l%DxV)HiY1O9smE%x34Li{gmL=xA2lX&6I=|pHH
z@u!U<7k=PtT=*sDjfupy7dQWg^7Fkv0gx3BCl-2Vur?0%oO;k_)hKLr0*16Y1-FS8
ztFJ<L=*{Ic7PVpjWf(GpNAB$dM&>*WgFDRiFF7saAb%8K(6zZTvGzD9XxKlG{A-`!
zb#7*2IsGj6qi|bjU*uFX@Zh}v<=xHA<3R5M`x8JhmSPD#J<39}jiG(@HPL6mYX<?t
zSau!wJCGsPI2da_-%vLbooA<8P8~gVkcT1UISbcS(8uSoKh}699N)R49~7b7cG>p8
zwg<L7u<e0u4}1U~p#4#4KSVagJ_t7iSQcEuVuF58(0V~>UoVbrrTyw&(NFubeWFck
z9Y1^kM*A~dAF)vXzuw&}V0&E*g!WK3#pZ~wq0rwWHfeZP!SYL&zz*3hxSnGXVz7Z!
zc$@Zie9Z=g)^~9{Wgp^PjLizegx?gKJR0Yc;DgO5FJuq41AP3~1wSnNyiI;x7i~xM
zzb)FOaSDAOG@)~|Z1?ZN4{r%N6px=&TH$1QL^-bZs9j1|N9VE5uFfu{b<{MJgzmU_
z+sBTukAgDS->%o>GnQ;C4uI>HD0f%r2>=YZf&{~O0tiLruF$txXjkY^Y<FEGwlB6T
z)Kt?5QyNy;>AjKI$+}Z@hxa1b*Zj>h_^Nip6=oksRiXdHcEX*10|E0PacW6HAC%fK
zS!x*$!(k-^E>Pm{>jR}NJ#CrORv~4=$;nXKa!xUC_D-64)5@fvsavV3-r1h+w(jFf
z+a$0*dM;kF+&)#vneFydMPSRckLmVPcDr3DS}9WlrG<8*U_6;=hx3=7%uMQ;Y&#yb
z>~=ha@oJl8>PDL}qg!clW@D;7mvi(aP{(rV6xH$t$5bctMKxK>WR13r!FX#*x2KqD
z%;iDSrH<wHTr@2^Q^@-o8uVFaR!0F*o6b5+<;POhF=wI952va*rKK%BXKGUh$WY7W
z)O5?z=Uh&zKbZnHd^FUrS_M89J(o#={Q_u(o61eW;ghMULM~_K0XEEJaT0Xs`AImr
zie@ICMtQd@nY7G{)XHY^Ce^_)?&SX;cZ`1+Ln!W4`rbq)L|C9EQ=#@2#0Zk7xJWc1
zI4OQmd?E$-odjbd$y3}UIxhkf+C?6cr+C%}80sc@io--H?g9%h6z8ao<HHz8@)Y-o
zDuR*5Pkcnjp&w%^wJEL=C3ze_+NSYCl@|;j^lS)F94ESjAQZAs^7Q<9=<~|cx<HiH
z5tPTZNS2=k497+57Ok5^X*~oMUeK1e*wFCGuZTEIl<bpCqo@%5q9k7$h#1g12}*JV
zUzX(8Nls8&|A=26|1(0K=C9Is(@OH!CHZEtAQLr6P*Cze!QX`n?qO(tX<tI$Rb_s;
z{bP`A@W|7?WnFB1WPZ8*bCAXPBl{cTyVS=2DF16op1xPn#!R*+xBme0I8|hy)_MAF
zzJh~;Li0xQ^!~gIectiYcd}dP6ez7ODcwn)=qu3Um8X3eeb9S9`By<6b<z0g`)aEg
zAlab-&~s3G6)I@Ig8)HE9``8TBHY1c5Wfc(VP3v5VPDo;#tT3h28u2V`5~{;09*AH
z<bN;ZD~+Gjwp5V+w~#+cjDpHzeOSoL&)K=l75G^NdD4SyG<ROs*M)p1F^W#9$$dw$
z!z<q`4i+~>JlqeR?QqfiK=B*nQ;_`?1v;QCO9rK;%T(bWfzi==?q!L<YDw#}ACHvI
zEq=U;(R%8~s~N46etZX`_0Nyjl-4&t9xa_`{CIhO2KythlF|C&=ZBqyi2HuLJYV7J
z*;&e$`T1jv;<q1fDCPhBcw_0j=*M?4il=_O34*@jg?K%}c0;}}2*00ET=NP=AYW99
zSDuat<b@~>d0_AxogTd?`jg&98K={-jLW+Mw?D$@G%fSfDS8XX_qdAP+uTCjT0M{u
zEB70YB#b*nX?}74iFk*E_X}M9j_^3wzvUnLKczWdK987}L;W`-{omsJ<?q8yz=NKv
z((^8HJux;*2YF~?`6FCGe!#vU{q8ze{$9P#^_0I?>l`o7H@+iq`Mqj_2MEgVVf<D=
zMlRie2jw$+xt{X>W4wZ%QNV-ppP%CT%k!U~t-!Afe))fq13c)tY`_)yp`iVh3ix*b
zKM=tGE5L*Dt1M+Xwo`<RQ3{T}=i{T=@VUpvAX~YW57nHUmcopv%_tG-fi%r1Xp`AO
zQqO9JQ?P7JFU~SJR!(P4$28RLW8Eh}a{v=hzPhJY$bLH39COdPMlqM0!*e)4_3{?a
z*7{i8R4I;maI}A9KpVJl7PGqi38-$^1#L>t8<@sD`<V;<Bk;*U(;&S|+(W~UoarCd
z9vK{bY+y_q>pwF*fI1*)i@M$`D=ZSYZu;1tS>`EU%;pL;-XyP^4-U!{qcQTXKLPDa
NGxzm*6T@y2`@a>G9{B(O

literal 0
HcmV?d00001

diff --git a/tests/data/test-read-dwarf/test3.so.abi b/tests/data/test-read-dwarf/test3.so.abi
new file mode 100644
index 0000000..f25683a
--- /dev/null
+++ b/tests/data/test-read-dwarf/test3.so.abi
@@ -0,0 +1,14 @@
+<abi-corpus path='data/test-read-dwarf/test3.so'>
+  <elf-function-symbols>
+    <elf-symbol name='__foo__' type='func-type' binding='global-binding' is-defined='yes'/>
+    <elf-symbol name='__foo' type='func-type' binding='global-binding' is-defined='yes'/>
+    <elf-symbol name='_fini' type='func-type' binding='global-binding' is-defined='yes'/>
+    <elf-symbol name='foo__' type='func-type' binding='weak-binding' is-defined='yes'/>
+    <elf-symbol name='_init' type='func-type' binding='global-binding' is-defined='yes'/>
+    <elf-symbol name='foo' type='func-type' binding='weak-binding' alias='foo__,__foo__,__foo' is-defined='yes'/>
+  </elf-function-symbols>
+  <abi-instr version='1.0' address-size='64' path='test3.c'>
+    <function-decl name='__foo' mangled-name='foo' filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test3.c' line='8' column='1' visibility='default' binding='global' size-in-bits='64' alignment-in-bits='64' elf-symbol-id='foo'>
+    </function-decl>
+  </abi-instr>
+</abi-corpus>
diff --git a/tests/test-read-dwarf.cc b/tests/test-read-dwarf.cc
index 8555954..d654c87 100644
--- a/tests/test-read-dwarf.cc
+++ b/tests/test-read-dwarf.cc
@@ -65,6 +65,11 @@ InOutSpec in_out_specs[] =
     "data/test-read-dwarf/test2.so.abi",
     "output/test-read-dwarf/test2.so.abi"
   },
+  {
+    "data/test-read-dwarf/test3.so",
+    "data/test-read-dwarf/test3.so.abi",
+    "output/test-read-dwarf/test3.so.abi"
+  },
   // This should be the last entry.
   {NULL, NULL, NULL}
 };
-- 
1.8.3.1