2012/8/25 Yann Rouillard <span dir="ltr"><<a href="mailto:yann@pleiades.fr.eu.org" target="_blank">yann@pleiades.fr.eu.org</a>></span><br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi everybody,<div><br></div><div>As discussed previously in the direct binding discussion, here is the new checkpkg test I am proposing to add to ensure direct binding is enabled in packages.</div><div>Of course the idea is to enable this check when we agree on enabling direct binding and after having tested it on a first set of package.</div>
<div><br></div><div>The full patch is attached, and I put an inline version without the unit test part for easy commenting.</div><div><br></div><div>Feedbacks are welcome.</div><div><br></div><div>Yann</div><div><br></div>
<div><div>diff -ur v2.orig//lib/python/dependency_checks.py v2/lib/python/dependency_checks.py</div><div>--- v2.orig//lib/python/dependency_checks.py<span style="white-space:pre-wrap"> </span>Thu Aug 16 12:05:42 2012</div>
<div>+++ v2/lib/python/dependency_checks.py<span style="white-space:pre-wrap"> </span>Sat Aug 25 16:14:45 2012</div><div>@@ -47,8 +47,8 @@</div><div> </div><div> def ProcessSoname(</div><div> ldd_emulator,</div>
<div>- soname, path_and_pkg_by_basename, binary_info, isalist, binary_path, logger,</div><div>- error_mgr,</div><div>+ soname, path_and_pkg_by_basename, binary_info, isalist, binary_path, needed_symbols,</div><div>
+ logger, error_mgr,</div><div> pkgname, messenger):</div><div> """This is not an ideal name for this function.</div><div> </div><div>@@ -57,6 +57,28 @@</div><div> """</div><div> logging.debug("ProcessSoname(), %s %s"</div>
<div> % (binary_info["path"], soname))</div><div>+</div><div>+ # Even when direct binding is enabled, some symbols might not be</div><div>+ # directly bound because the library explicitely requested the symbol</div>
<div>+ # not to be drectly bound to.</div><div>+ # For example, libc.so.1 does it for symbol sigaction, free, malloc and realloc</div><div>+ # So we consider that direct binding is enabled if at least one symbol is directly</div>
<div>+ # bound to because that definitely means that -B direct or -z direct was used.</div><div>+ direct_bind = False</div><div>+ for syminfo in needed_symbols:</div><div>+ if ('D' in syminfo['flags'] and 'B' in syminfo['flags']):</div>
</div></blockquote><div><br></div><div>The parentheses are not necessary.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>
<div>+ direct_bind = True</div><div>+ break</div><div>+</div><div>+ if not direct_bind: </div><div>+ messenger.Message(</div><div>+ "No symbol of binary %s is directly binded against library %s. "</div>
<div>+ "Please make sure the binaries are compiled using the \"-Bdirect\" linker option."</div><div>+ % ("/" + binary_info["path"], soname))</div><div>+ error_mgr.ReportError(</div>
<div>+ pkgname, "no-direct-binding",</div><div>+ "%s is not directly binded to soname %s" % ("/" + binary_info["path"], soname))</div><div>+</div><div> orphan_sonames = []</div>
<div> resolved = False</div><div> path_list = path_and_pkg_by_basename[soname].keys()</div><div>@@ -146,11 +168,22 @@</div><div> orphan_sonames = []</div><div> for binary_info in pkg_data["binaries_dump_info"]:</div>
<div> binary_path, binary_basename = os.path.split(binary_info["path"])</div><div>+</div><div>+ binary_needed_symbols = pkg_data["needed_symbols"][binary_info["path"]]</div></div></blockquote>
<div><br></div><div>We need to think whether we will reindex all packages, or we'll accept older data structures without throwing IndexError in the line above.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div> for soname in binary_info["needed sonames"]:</div>
<div>+ if soname in binary_needed_symbols:</div><div>+ needed_symbols = binary_needed_symbols[soname]</div><div>+ else:</div><div>+<span style="white-space:pre-wrap"> </span># If we are here, that means we got no information on symbols.</div>
<div>+<span style="white-space:pre-wrap"> </span># No syminfo section was present in the elf file, that definitely</div><div>+<span style="white-space:pre-wrap"> </span># imply that -Bdirect or -z direct was not used and the subsequent</div>
<div>+<span style="white-space:pre-wrap"> </span># check on direct binding will catch it because it will find not symbols</div><div>+<span style="white-space:pre-wrap"> </span>needed_symbols = []</div>
<div>+</div><div> orphan_sonames_tmp = ProcessSoname(</div><div> ldd_emulator,</div><div>- soname, path_and_pkg_by_basename, binary_info, isalist, binary_path, logger,</div><div>- error_mgr,</div>
<div>+ soname, path_and_pkg_by_basename, binary_info, isalist, binary_path, needed_symbols,</div><div>+<span style="white-space:pre-wrap"> </span> logger, error_mgr,</div><div> pkgname, messenger)</div>
<div> orphan_sonames.extend(orphan_sonames_tmp)</div><div> </div><div>diff -ur v2.orig//lib/python/dependency_checks_test.py v2/lib/python/dependency_checks_test.py</div><div>--- v2.orig//lib/python/dependency_checks_test.py<span style="white-space:pre-wrap"> </span>Tue Jun 5 23:52:51 2012</div>
<div>+++ v2/lib/python/dependency_checks_test.py<span style="white-space:pre-wrap"> </span>Sat Aug 25 13:37:57 2012</div><div>@@ -258,6 +258,8 @@</div><div> '/opt/csw/share/man': [u'CSWcommon', u'CSWgnuplot'],</div>
<div> '/opt/csw/share/man/man1': ['CSWtree'],</div><div> '/opt/csw/share/man/man1/tree.1': ['CSWtree']}</div><div>+ self.error_mgr_mock.ReportError('CSWtree', 'no-direct-binding', </div>
<div>+<span style="white-space:pre-wrap"> </span> '/opt/csw/bin/tree is not directly binded to soname libc.so.1') </div><div> self.error_mgr_mock.NeedFile('CSWtree', u'/usr/lib/libc.so.1',</div>
<div> 'opt/csw/bin/tree needs the libc.so.1 soname')</div><div> self.mox.ReplayAll()</div><div>diff -ur v2.orig//lib/python/inspective_package.py v2/lib/python/inspective_package.py</div><div>--- v2.orig//lib/python/inspective_package.py<span style="white-space:pre-wrap"> </span>Thu Aug 23 23:56:57 2012</div>
<div>+++ v2/lib/python/inspective_package.py<span style="white-space:pre-wrap"> </span>Sat Aug 25 16:17:17 2012</div><div>@@ -229,6 +229,45 @@</div><div> </div><div> return defined_symbols</div><div>
</div><div>+</div><div>+ def GetNeededSymbols(self):</div><div>+ """Returns symbols needed for packaged ELF objects to work</div><div>+</div><div>+ To do this we parse output lines from elfdump -y, that command</div>
<div>+ will give us the content of the .SUNW_syminfo section which </div><div>+ contains additionnal informations on symbols linking.</div><div>+ """</div><div>+ binaries = self.ListBinaries()</div>
<div>+ needed_symbols = {}</div><div>+</div><div>+ for binary in binaries:</div><div>+ binary_abspath = os.path.join(self.directory, "root", binary)</div><div>+ # elfdump is the only tool that give us the syminfo flags</div>
<div>+ args = ["/usr/ccs/bin/elfdump", "-y", binary_abspath]</div><div>+ elfdump_proc = subprocess.Popen(</div><div>+ args,</div><div>+ stdout=subprocess.PIPE,</div><div>+ stderr=subprocess.PIPE)</div>
<div>+ stdout, stderr = elfdump_proc.communicate()</div><div>+ retcode = elfdump_proc.wait()</div><div>+ if retcode:</div><div>+ logging.error("%s returned an error: %s", args, stderr)</div>
<div>+ continue</div><div>+ elfdump_out = stdout.splitlines()</div><div>+</div><div>+ needed_symbols[binary] = {}</div><div>+ for line in elfdump_out:</div><div>+ syminfo = self._ParseElfdumpSyminfoLine(line)</div>
<div>+ if not syminfo:</div><div>+ continue</div></div></blockquote><div><br></div><div>In what circumstances does syminfo come back empty? Is it when we can't parse the text output?</div><div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>+<span style="white-space:pre-wrap"> </span>if syminfo['library']:</div><div>+ library = syminfo['library']<span style="white-space:pre-wrap"> </span> </div>
<div>+ del syminfo['library']</div></div></blockquote><div><br></div><div>Will the del above actually free anything? If the whole syminfo stays around, all its elements will too.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div>+<span style="white-space:pre-wrap"> </span> symbols = needed_symbols[binary].setdefault(library, [])</div><div>+<span style="white-space:pre-wrap"> </span> symbols.append(syminfo)</div>
<div>+</div><div>+ return needed_symbols</div><div>+</div><div> def GetLddMinusRlines(self):</div><div> """Returns ldd -r output."""</div><div> binaries = self.ListBinaries()</div>
<div>@@ -274,6 +313,22 @@</div><div> sym = { 'address': fields[0], 'type': fields[1], 'name': fields[2] }</div><div> return sym</div><div> </div><div>+ def _ParseElfdumpSyminfoLine(self, line):</div>
<div>+ blank_line = (r'^\s*$')</div><div>+ header_line = (r'^(?:Syminfo Section:\s+.SUNW_syminfo|\s*index\s+flags\s+bound\s+to\s+symbol)\s*$')</div><div>+ needed_symbol = (r'\s*\[\d+\]\s+(?P<flags>[ABCDFILNPS]+)\s*(\[\d+\]\s+(?P<library>.*\S)|<self>)?\s+(?P<symbol>.*\S)\s*')</div>
<div>+ common_re = (r"(?:%s|%s|%s)" % (blank_line, header_line, needed_symbol))</div><div>+</div><div>+ m = re.match(common_re, line)</div><div>+ if not m:</div><div>+ raise package.StdoutSyntaxError("Could not parse %s with %s"</div>
<div>+ % (repr(line), common_re))</div><div>+ syminfo = m.groupdict()</div><div>+ if 'library' in syminfo and syminfo['library']:</div><div>+ return syminfo</div>
<div>+ else:</div><div>+ return None</div><div>+</div><div> def _ParseLddDashRline(self, line, binary=None):</div><div> found_re = r"^\t(?P<soname>\S+)\s+=>\s+(?P<path_found>\S+)"</div>
<div> symbol_not_found_re = (r"^\tsymbol not found:\s(?P<symbol>\S+)\s+"</div><div>diff -ur v2.orig//lib/python/package_stats.py v2/lib/python/package_stats.py</div><div>--- v2.orig//lib/python/package_stats.py<span style="white-space:pre-wrap"> </span>Fri Aug 10 02:12:10 2012</div>
<div>+++ v2/lib/python/package_stats.py<span style="white-space:pre-wrap"> </span>Sat Aug 25 15:58:49 2012</div><div>@@ -209,6 +209,7 @@</div><div> "files_metadata": dir_pkg.GetFilesMetadata(),</div>
<div> "mtime": self.GetMtime(),</div><div> <span style="white-space:pre-wrap"> </span>"ldd_info": dir_pkg.GetLddMinusRlines(),</div><div>+<span style="white-space:pre-wrap"> </span>"needed_symbols": dir_pkg.GetNeededSymbols(),</div>
<div> }</div><div> self.SaveStats(pkg_stats)</div><div> logging.debug("Statistics of %s have been collected.", repr(dir_pkg.pkgname))</div></div></blockquote><div><br></div><div><br></div></div>