From 20ceacaff62f6bf3e7c08128ea03d6faa84c880c Mon Sep 17 00:00:00 2001 From: Alexey Alexandrov Date: Wed, 14 Feb 2018 09:16:22 -0800 Subject: [PATCH] Enable MacOS binutils test. (#322) --- .travis.yml | 7 ++-- internal/binutils/addr2liner.go | 25 +++++++++----- internal/binutils/binutils.go | 11 +++++-- internal/binutils/binutils_test.go | 4 +-- internal/binutils/testdata/build_mac.sh | 31 ++++++++++++++++++ internal/binutils/testdata/exe_mac_64 | Bin 0 -> 8648 bytes .../exe_mac_64.dSYM/Contents/Info.plist | 20 +++++++++++ .../Contents/Resources/DWARF/exe_mac_64 | Bin 0 -> 8840 bytes internal/binutils/testdata/lib_mac_64 | Bin 0 -> 4496 bytes .../lib_mac_64.dSYM/Contents/Info.plist | 20 +++++++++++ .../Contents/Resources/DWARF/lib_mac_64 | Bin 0 -> 8934 bytes 11 files changed, 100 insertions(+), 18 deletions(-) create mode 100755 internal/binutils/testdata/build_mac.sh create mode 100755 internal/binutils/testdata/exe_mac_64 create mode 100644 internal/binutils/testdata/exe_mac_64.dSYM/Contents/Info.plist create mode 100644 internal/binutils/testdata/exe_mac_64.dSYM/Contents/Resources/DWARF/exe_mac_64 create mode 100755 internal/binutils/testdata/lib_mac_64 create mode 100644 internal/binutils/testdata/lib_mac_64.dSYM/Contents/Info.plist create mode 100644 internal/binutils/testdata/lib_mac_64.dSYM/Contents/Resources/DWARF/lib_mac_64 diff --git a/.travis.yml b/.travis.yml index b8e07e35..c7927a0f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -47,10 +47,11 @@ addons: apt: packages: - graphviz - + before_install: - - go get -u github.com/golang/lint/golint honnef.co/go/tools/cmd/... - - if [[ "$TRAVIS_OS_NAME" == "osx" && -z $SKIP_GRAPHVIZ ]]; then brew update ; fi + - go get -u github.com/golang/lint/golint honnef.co/go/tools/cmd/... + - if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then brew update ; fi + - if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then brew install binutils ; fi - if [[ "$TRAVIS_OS_NAME" == "osx" && -z $SKIP_GRAPHVIZ ]]; then brew install graphviz; fi script: diff --git a/internal/binutils/addr2liner.go b/internal/binutils/addr2liner.go index 71e471b5..c0661bf4 100644 --- a/internal/binutils/addr2liner.go +++ b/internal/binutils/addr2liner.go @@ -41,9 +41,11 @@ type addr2Liner struct { rw lineReaderWriter base uint64 - // nm holds an NM based addr2Liner which can provide - // better full names compared to addr2line, which often drops - // namespaces etc. from the names it returns. + // nm holds an addr2Liner using nm tool. Certain versions of addr2line + // produce incomplete names due to + // https://sourceware.org/bugzilla/show_bug.cgi?id=17541. As a workaround, + // the names from nm are used when they look more complete. See addrInfo() + // code below for the exact heuristic. nm *addr2LinerNM } @@ -215,17 +217,22 @@ func (d *addr2Liner) addrInfo(addr uint64) ([]plugin.Frame, error) { return nil, err } - // Get better name from nm if possible. + // Certain versions of addr2line produce incomplete names due to + // https://sourceware.org/bugzilla/show_bug.cgi?id=17541. Attempt to replace + // the name with a better one from nm. if len(stack) > 0 && d.nm != nil { nm, err := d.nm.addrInfo(addr) if err == nil && len(nm) > 0 { - // Last entry in frame list should match since - // it is non-inlined. As a simple heuristic, - // we only switch to the nm-based name if it - // is longer. + // Last entry in frame list should match since it is non-inlined. As a + // simple heuristic, we only switch to the nm-based name if it is longer + // by 2 or more characters. We consider nm names that are longer by 1 + // character insignificant to avoid replacing foo with _foo on MacOS (for + // unknown reasons read2line produces the former and nm produces the + // latter on MacOS even though both tools are asked to produce mangled + // names). nmName := nm[len(nm)-1].Func a2lName := stack[len(stack)-1].Func - if len(nmName) > len(a2lName) { + if len(nmName) > len(a2lName)+1 { stack[len(stack)-1].Func = nmName } } diff --git a/internal/binutils/binutils.go b/internal/binutils/binutils.go index 390f952f..94edd071 100644 --- a/internal/binutils/binutils.go +++ b/internal/binutils/binutils.go @@ -111,6 +111,11 @@ func initTools(b *binrep, config string) { defaultPath := paths[""] b.llvmSymbolizer, b.llvmSymbolizerFound = findExe("llvm-symbolizer", append(paths["llvm-symbolizer"], defaultPath...)) b.addr2line, b.addr2lineFound = findExe("addr2line", append(paths["addr2line"], defaultPath...)) + if !b.addr2lineFound { + // On MacOS, brew installs addr2line under gaddr2line name, so search for + // that if the tool is not found by its default name. + b.addr2line, b.addr2lineFound = findExe("gaddr2line", append(paths["addr2line"], defaultPath...)) + } b.nm, b.nmFound = findExe("nm", append(paths["nm"], defaultPath...)) b.objdump, b.objdumpFound = findExe("objdump", append(paths["objdump"], defaultPath...)) } @@ -306,9 +311,9 @@ func (f *fileNM) SourceLine(addr uint64) ([]plugin.Frame, error) { } // fileAddr2Line implements the binutils.ObjFile interface, using -// 'addr2line' to map addresses to symbols (with file/line number -// information). It can be slow for large binaries with debug -// information. +// llvm-symbolizer, if that's available, or addr2line to map addresses to +// symbols (with file/line number information). It can be slow for large +// binaries with debug information. type fileAddr2Line struct { once sync.Once file diff --git a/internal/binutils/binutils_test.go b/internal/binutils/binutils_test.go index 0317cf51..9f117196 100644 --- a/internal/binutils/binutils_test.go +++ b/internal/binutils/binutils_test.go @@ -265,8 +265,6 @@ func TestObjFile(t *testing.T) { func TestMachoFiles(t *testing.T) { skipUnlessDarwinAmd64(t) - t.Skip("Disabled because of issues with addr2line (see https://github.com/google/pprof/pull/313#issuecomment-364073010)") - // Load `file`, pretending it was mapped at `start`. Then get the symbol // table. Check that it contains the symbol `sym` and that the address // `addr` gives the `expected` stack trace. @@ -291,7 +289,7 @@ func TestMachoFiles(t *testing.T) { {"lib normal mapping", "lib_mac_64", 0, math.MaxUint64, 0, 0xfa0, "_bar", []plugin.Frame{ - {Func: "bar", File: "/tmp/lib.c", Line: 6}, + {Func: "bar", File: "/tmp/lib.c", Line: 5}, }}, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/binutils/testdata/build_mac.sh b/internal/binutils/testdata/build_mac.sh new file mode 100755 index 00000000..5ec98f39 --- /dev/null +++ b/internal/binutils/testdata/build_mac.sh @@ -0,0 +1,31 @@ +#!/bin/bash -x + +# This is a script that generates the test MacOS executables in this directory. +# It should be needed very rarely to run this script. It is mostly provided +# as a future reference on how the original binary set was created. + +set -o errexit + +cat </tmp/hello.cc +#include + +int main() { + printf("Hello, world!\n"); + return 0; +} +EOF + +cat </tmp/lib.c +int foo() { + return 1; +} + +int bar() { + return 2; +} +EOF + +cd $(dirname $0) +rm -rf exe_mac_64* lib_mac_64* +clang -g -o exe_mac_64 /tmp/hello.c +clang -g -o lib_mac_64 -dynamiclib /tmp/lib.c diff --git a/internal/binutils/testdata/exe_mac_64 b/internal/binutils/testdata/exe_mac_64 new file mode 100755 index 0000000000000000000000000000000000000000..dba1ae15817595eb443322f6ae4ce9e67e400b90 GIT binary patch literal 8648 zcmeHNO=}ZD7@k-^>Sxo7_|3LhsiN7m)mjm;RfAEBR;?ff8Ji|G!A(+kx7zBZQalt9 zdiLnqga5#Tf(n9y7k_|?g5E`n@p)!vo844B3c|eb?(8%3&NDO5%tChGy!r9%=PDr* zO+w5p6GF7WpDY*Rp0IEy#AbLFF6Gd`$;?RR!a3H)EfI&mMdW#pKq-eamxtpas(&(G zCp5;~#0u?Id|NaDF?C8F zt2%PRcmK#2((rt+e`wTSji)Q5bVmQimqdTYH|u!fC?C&V6xW}F!Bc0?WKNt4&#=|7 zPGH(gJsI*u=hcqqEX@Eb7`qweCSq-tCDhSRqhuVlXL}KN9m>8oA?8u8g%81LTmzl4 zYjla?x7LaNvFAZYZ9r+kIfgyyxwccv+4*TF4@}z%=RAhqy>9R6Kl=3kqlZqd>-6fK zs7u1ddW;O`bEh!tV4Ya6>ep~C)mqTbw%~kyz_b$eu`&L|Y0oc|Q^!)6M=3`(A$GyZ zf7J@syjx@HW6UUE6fg=H1&jhl0i%FXz$oyq6}Xh0`r+(0>B#6^7hRIAmp1;?rGwQf~iC*Q`O z_3Axh8*gCIlE0t%p1U6<{f_>v&3nGXtV7SF>J1Qo9Z?n0%RKrfq7~-eC}0#Y3K#{9 z0!9I&fKk9GU=%P47zK<1MuGocf!6lqc`l;0pUFOz43|0NhmpKK?;_LaCR_XE1QO8x zxXL?j{CZLd-r$uq^cqqOif9MU;o6~sQ>@~ literal 0 HcmV?d00001 diff --git a/internal/binutils/testdata/exe_mac_64.dSYM/Contents/Info.plist b/internal/binutils/testdata/exe_mac_64.dSYM/Contents/Info.plist new file mode 100644 index 00000000..b6f8ea3f --- /dev/null +++ b/internal/binutils/testdata/exe_mac_64.dSYM/Contents/Info.plist @@ -0,0 +1,20 @@ + + + + + CFBundleDevelopmentRegion + English + CFBundleIdentifier + com.apple.xcode.dsym.exe_mac_64 + CFBundleInfoDictionaryVersion + 6.0 + CFBundlePackageType + dSYM + CFBundleSignature + ???? + CFBundleShortVersionString + 1.0 + CFBundleVersion + 1 + + diff --git a/internal/binutils/testdata/exe_mac_64.dSYM/Contents/Resources/DWARF/exe_mac_64 b/internal/binutils/testdata/exe_mac_64.dSYM/Contents/Resources/DWARF/exe_mac_64 new file mode 100644 index 0000000000000000000000000000000000000000..2cb0e3bf31e384571a684c72d0b942ba195c969c GIT binary patch literal 8840 zcmeI2&u`R56vxNASsGZngx|FE;L=nI2-eGw0|}^P#ik^XmWYO*myx|YY>d3NWqTn( zNX?-Ks;Y-xbF8=^^@`M6{{bWpz4TAu(gPAFkf864-)tT4E^JTbFfW>U^W)9OQ!i1%e$ErInqVol zJa1_S+&pw0Q32q`GG(Z&XRDmYN!N*cO?lpZ;%(_gq*bu>a*T3ibE$;fa16kj^9={O}*IZeiq1w^o zx;}@*d858u#}+bJ&-bk>?oq|&-1m6A)^P%lhed-l6{6EweZ z1puGr;l9h(|9uV?7rwnT*H}<{*v#E0P4Fynk&~Sla()Le=ykn*xaa)bVt`{7k&5rPac+m9C%9Z(h{^Mw5KryfC;x>P+RZ%g1@R_nJ}*Y($;~og zetS=(_j{Rmdk;}zh*S3)ICq*DlAm9TcxryF_wa^XMmK8(v;tZIt$m zKr5gX&4c2I$+Q<#o9UtC)`I#j zu%}eqHh%M$LK#Nvmkh`lo_^s;dhhc9eojbFB(du}B-c+*LOY<%Qkn;NZ15$%>V)%4K%~?g;-qCKcB(4sH~(FIo>ob&%XYXgw6rb#*Md9WZ#+Pac@=B(#61fEb3RkMU_N2^6!lJZMP-FCzQ*g8jO{t*P4>Z7(>^ICrdzWa|}q+EvwP*E`rq_IsoIt?MFUbC-A@v*tUQtJH&y&HHPs8|JO%eTjLPiYoa| zy$l1B80X*_xeA)yX!-XM=jM>XyR+v!FrPLvEz-w)9{vzc)G@p;kEcHx_(83ezniZPfRG5g4rkvShO?M&Xv}w7vkX`UECZGS%YbFTGGH073|Iy% z1OGDvkJn#+en;W9b(G1jiiDVgYU?PJoU{7z@tNDy%aoRMqkXxiuAhJon;NOXKV@M` zL8p(^I~|$!DqXplmSGlEhglTDhS^AnUBS8U~Jr%T< i`X#^BzxCX8i(PlcDN_h9t`tfocRAmQ`iCd!d;SJVA9g|j literal 0 HcmV?d00001 diff --git a/internal/binutils/testdata/lib_mac_64.dSYM/Contents/Info.plist b/internal/binutils/testdata/lib_mac_64.dSYM/Contents/Info.plist new file mode 100644 index 00000000..ad5e0204 --- /dev/null +++ b/internal/binutils/testdata/lib_mac_64.dSYM/Contents/Info.plist @@ -0,0 +1,20 @@ + + + + + CFBundleDevelopmentRegion + English + CFBundleIdentifier + com.apple.xcode.dsym.lib_mac_64 + CFBundleInfoDictionaryVersion + 6.0 + CFBundlePackageType + dSYM + CFBundleSignature + ???? + CFBundleShortVersionString + 1.0 + CFBundleVersion + 1 + + diff --git a/internal/binutils/testdata/lib_mac_64.dSYM/Contents/Resources/DWARF/lib_mac_64 b/internal/binutils/testdata/lib_mac_64.dSYM/Contents/Resources/DWARF/lib_mac_64 new file mode 100644 index 0000000000000000000000000000000000000000..e466c1342e3f5be57c4aa21e2745bcdd21b1e817 GIT binary patch literal 8934 zcmeI2&rj4q6vwCgLxJJ~k|2sP)nI}~wi|y%&sdm{^t(#Tqi;(qM^%X{7t&tZPTGNOZRhOF1RlvdnT+bvPsUGiWencpkU zo2cD>dMnLG2hfmGYqyka_sen{KDj*e*3P=T{0#X5~fK)&# zAQjk+3fzLEQv?tD6y;}bAJhsw{o@g}qDRX`KPdvtnEKa1KLY2)BrAs-U>&~Wg zod^1IUCRDFa7~_IXS6q+8&)r>Q{5wQvY+lJ>D*;iWx0;wd8;-(J%2-6v4YU{Tx}>{ z$QQJMq625xgF^*y@@Izfr%w=+UXPX6=R%0q4a2e447a40Jv|JHdfATdS72wnWf<|w zj-~s4;4SGOqZPP@qnox{iEJm-BP)zbMr5!(K>bna$vb0{xX-9G7W{MrE#7-r5AVNQ z+)$r{8!BptpF1!upvApJW1>NedyFQy$U#eTe}mP=U>ts4f`+@MkUj8OLl*v2=*4*y a*5mu5_!y5n0msNM2ZkBF76&s8Z}|rxW9=pY literal 0 HcmV?d00001