From 9c932f1d0c5667846124663e738656db73595645 Mon Sep 17 00:00:00 2001 From: "rsesek@chromium.org" Date: Thu, 31 Jul 2014 19:11:29 +0000 Subject: [PATCH] upload_system_symbols: Use the Go1.3 improvements to debug/macho. This removes the custom MachO header reading functionality, since the stdlib can now read Fat files. R=andybons@chromium.org, mark@chromium.org Review URL: https://breakpad.appspot.com/10684002 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1360 4c0a9323-5329-0410-9bdc-e9ce6186880e --- .../upload_system_symbols/arch_constants.h | 28 +- .../mac/upload_system_symbols/arch_reader.go | 239 ++---------------- .../upload_system_symbols/arch_reader_test.go | 82 ------ .../upload_system_symbols/testdata/Makefile | 22 -- .../upload_system_symbols/testdata/archtest.c | 7 - .../upload_system_symbols.go | 51 +++- 6 files changed, 80 insertions(+), 349 deletions(-) delete mode 100644 src/tools/mac/upload_system_symbols/arch_reader_test.go delete mode 100644 src/tools/mac/upload_system_symbols/testdata/Makefile delete mode 100644 src/tools/mac/upload_system_symbols/testdata/archtest.c diff --git a/src/tools/mac/upload_system_symbols/arch_constants.h b/src/tools/mac/upload_system_symbols/arch_constants.h index ed318b24..2115e54e 100644 --- a/src/tools/mac/upload_system_symbols/arch_constants.h +++ b/src/tools/mac/upload_system_symbols/arch_constants.h @@ -28,19 +28,33 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include +#include #include +#include // Go/Cgo does not support #define constants, so turn them into symbols // that are reachable from Go. -const cpu_type_t kCPUType_i386 = CPU_TYPE_I386; -const cpu_type_t kCPUType_x86_64 = CPU_TYPE_X86_64; +#ifndef CPU_TYPE_ARM64 +#define CPU_TYPE_ARM64 (CPU_TYPE_ARM | CPU_ARCH_ABI64) +#endif -const uint32_t kMachHeaderMagic32 = MH_MAGIC; -const uint32_t kMachHeaderMagic64 = MH_MAGIC_64; -const uint32_t kMachHeaderMagicFat = FAT_MAGIC; -const uint32_t kMachHeaderCigamFat = FAT_CIGAM; +#ifndef CPU_SUBTYPE_ARM64_ALL +#define CPU_SUBTYPE_ARM64_ALL 0 +#endif + +const cpu_type_t kCPU_TYPE_ARM = CPU_TYPE_ARM; +const cpu_type_t kCPU_TYPE_ARM64 = CPU_TYPE_ARM64; + +const cpu_subtype_t kCPU_SUBTYPE_ARM64_ALL = CPU_SUBTYPE_ARM64_ALL; +const cpu_subtype_t kCPU_SUBTYPE_ARM_V7S = CPU_SUBTYPE_ARM_V7S; + +const char* GetNXArchInfoName(cpu_type_t cpu_type, cpu_subtype_t cpu_subtype) { + const NXArchInfo* arch_info = NXGetArchInfoFromCpuType(cpu_type, cpu_subtype); + if (!arch_info) + return 0; + return arch_info->name; +} const uint32_t kMachHeaderFtypeDylib = MH_DYLIB; const uint32_t kMachHeaderFtypeBundle = MH_BUNDLE; diff --git a/src/tools/mac/upload_system_symbols/arch_reader.go b/src/tools/mac/upload_system_symbols/arch_reader.go index 0468836e..f6064823 100644 --- a/src/tools/mac/upload_system_symbols/arch_reader.go +++ b/src/tools/mac/upload_system_symbols/arch_reader.go @@ -31,238 +31,35 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. package main import ( - "encoding/binary" - "errors" - "fmt" - "os" - "reflect" - "unsafe" + "debug/macho" ) /* -#include -#include -#include - #include "arch_constants.h" */ import "C" -var ( - ErrNotMachO = errors.New("GetMachOImageInfo: file is not a supported Mach-O image") - ErrUnsupportedArch = errors.New("GetMachOImageInfo: unknown architecture detected") -) - -const ( - ArchI386 = "i386" - ArchX86_64 = "x86_64" -) - -type MachOType int - -const ( - MachODylib MachOType = C.kMachHeaderFtypeDylib - MachOBundle = C.kMachHeaderFtypeBundle - MachOExe = C.kMachHeaderFtypeExe -) - -type ImageInfo struct { - Type MachOType - Arch string -} - -// GetMachOImageInfo will read the file at filepath and determine if it is -// Mach-O. If it is, it will return a slice of ImageInfo that describe the -// images in the file (may be more than one if it is a fat image). -func GetMachOImageInfo(filepath string) ([]ImageInfo, error) { - f, err := os.Open(filepath) - if err != nil { - return nil, err +// getArchStringFromHeader takes a MachO FileHeader and returns a string that +// represents the CPU type and subtype. +// This function is a Go version of src/common/mac/arch_utilities.cc:BreakpadGetArchInfoFromCpuType(). +func getArchStringFromHeader(header macho.FileHeader) string { + // TODO(rsesek): As of 10.9.4, OS X doesn't list these in /usr/include/mach/machine.h. + if header.Cpu == C.kCPU_TYPE_ARM64 && header.SubCpu == C.kCPU_SUBTYPE_ARM64_ALL { + return "arm64" } - defer f.Close() - - // Read the magic number to determine the type of file this is. - var magic uint32 - err = binary.Read(f, binary.LittleEndian, &magic) - if err != nil { - return nil, err + if header.Cpu == C.kCPU_TYPE_ARM && header.SubCpu == C.kCPU_SUBTYPE_ARM_V7S { + return "armv7s" } - // Rewind the file since the magic number is a field in the header - // structs. - f.Seek(0, os.SEEK_SET) - - switch magic { - case C.kMachHeaderMagic32: - return readThinHeader(f, C.kMachHeaderMagic32) - case C.kMachHeaderMagic64: - return readThinHeader(f, C.kMachHeaderMagic64) - case C.kMachHeaderCigamFat: // Fat header is big-endian but was read in little. - return readFatHeader(f) - } - - return nil, ErrNotMachO -} - -func readThinHeader(f *os.File, expectedMagic uint32) ([]ImageInfo, error) { - var ( - magic, filetype uint32 - cpu C.cpu_type_t - err error - ) - - if expectedMagic == C.kMachHeaderMagic32 { - magic, cpu, filetype, err = readThin32Header(f) - } else if expectedMagic == C.kMachHeaderMagic64 { - magic, cpu, filetype, err = readThin64Header(f) - } else { - panic(fmt.Sprintf("Unexpected magic %#x", magic)) - } - if err != nil { - return nil, err - } - - if magic != expectedMagic { - return nil, fmt.Errorf("readThinHeader: unexpected magic number %#x", magic) - } - - arch := cpuTypeToArch(cpu) - if arch == "" { - return nil, ErrUnsupportedArch - } - return []ImageInfo{{MachOType(filetype), arch}}, nil -} - -func readThin32Header(f *os.File) (uint32, C.cpu_type_t, uint32, error) { - var machHeader C.struct_mach_header - err := readStruct(f, binary.LittleEndian, unsafe.Pointer(&machHeader), C.struct_mach_header{}) - if err != nil { - return 0, 0, 0, err - } - return uint32(machHeader.magic), machHeader.cputype, uint32(machHeader.filetype), nil -} - -func readThin64Header(f *os.File) (uint32, C.cpu_type_t, uint32, error) { - var machHeader C.struct_mach_header_64 - err := readStruct(f, binary.LittleEndian, unsafe.Pointer(&machHeader), C.struct_mach_header_64{}) - if err != nil { - return 0, 0, 0, err - } - return uint32(machHeader.magic), machHeader.cputype, uint32(machHeader.filetype), nil -} - -func readFatHeader(f *os.File) ([]ImageInfo, error) { - var fatHeader C.struct_fat_header - err := readStruct(f, binary.BigEndian, unsafe.Pointer(&fatHeader), C.struct_fat_header{}) - if err != nil { - return nil, err - } - - if fatHeader.magic != C.kMachHeaderMagicFat { - return nil, fmt.Errorf("readFatHeader: unexpected magic number %#x", fatHeader.magic) - } - - // Read the fat_arch headers. - headers := make([]C.struct_fat_arch, fatHeader.nfat_arch) - for i := 0; i < int(fatHeader.nfat_arch); i++ { - var fatArch C.struct_fat_arch - err = readStruct(f, binary.BigEndian, unsafe.Pointer(&fatArch), C.struct_fat_arch{}) - if err != nil { - return nil, fmt.Errorf("readFatHeader: %v", err) - } - headers[i] = fatArch - } - - seenArches := make(map[string]int) - - // Now go to each arch in the fat image and read its mach header. - infos := make([]ImageInfo, 0, len(headers)) - for _, header := range headers { - f.Seek(int64(header.offset), os.SEEK_SET) - - var thinarch []ImageInfo - var expectedArch string - switch header.cputype { - case C.kCPUType_i386: - thinarch, err = readThinHeader(f, C.kMachHeaderMagic32) - expectedArch = ArchI386 - case C.kCPUType_x86_64: - thinarch, err = readThinHeader(f, C.kMachHeaderMagic64) - expectedArch = ArchX86_64 - default: - err = ErrUnsupportedArch - } - - if err != nil { - return nil, err - } - if thinarch[0].Arch != expectedArch { - return nil, fmt.Errorf("readFatHeader: expected arch %d, got %d", thinarch[0].Arch, expectedArch) - } - - infos = append(infos, thinarch[0]) - seenArches[thinarch[0].Arch]++ - } - - for arch, count := range seenArches { - if count != 1 { - return nil, fmt.Errorf("readFatHeader: duplicate arch %s detected", arch) - } - } - - return infos, nil -} - -// TODO(rsesek): Support more arches. -func cpuTypeToArch(cpu C.cpu_type_t) string { - switch cpu { - case C.kCPUType_i386: - return ArchI386 - case C.kCPUType_x86_64: - return ArchX86_64 - default: + cstr := C.GetNXArchInfoName(C.cpu_type_t(header.Cpu), C.cpu_subtype_t(header.SubCpu)) + if cstr == nil { return "" } + return C.GoString(cstr) } -// readStruct is a incomplete version of binary.Read that uses unsafe pointers -// to set values in unexported fields. From |f|, this will read the fields of -// the |destType| template instance, in the specified byte |order|, and place -// the resulting memory into |dest|. -func readStruct(f *os.File, order binary.ByteOrder, dest unsafe.Pointer, destType interface{}) error { - rv := reflect.ValueOf(destType) - rt := rv.Type() - destPtr := uintptr(dest) - - for i := 0; i < rv.NumField(); i++ { - field := rv.Field(i) - fieldType := rt.Field(i) - - var vp unsafe.Pointer - var err error - - switch field.Kind() { - case reflect.Int32: - var v int32 - vp = unsafe.Pointer(&v) - err = binary.Read(f, order, &v) - case reflect.Uint32: - var v uint32 - vp = unsafe.Pointer(&v) - err = binary.Read(f, order, &v) - default: - err = fmt.Errorf("readStruct: unsupported type %v", fieldType) - } - - if err != nil { - return err - } - - memcpy(destPtr+fieldType.Offset, vp, fieldType.Type.Size()) - } - return nil -} - -func memcpy(dest uintptr, value unsafe.Pointer, size uintptr) { - C.memcpy(unsafe.Pointer(dest), value, C.size_t(size)) -} +const ( + MachODylib macho.Type = C.kMachHeaderFtypeDylib + MachOBundle = C.kMachHeaderFtypeBundle + MachOExe = C.kMachHeaderFtypeExe +) diff --git a/src/tools/mac/upload_system_symbols/arch_reader_test.go b/src/tools/mac/upload_system_symbols/arch_reader_test.go deleted file mode 100644 index 5fa1d210..00000000 --- a/src/tools/mac/upload_system_symbols/arch_reader_test.go +++ /dev/null @@ -1,82 +0,0 @@ -/* Copyright 2014, Google Inc. -All rights reserved. - -Redistribution and use in source and binary forms, with or without -modification, are permitted provided that the following conditions are -met: - - * Redistributions of source code must retain the above copyright -notice, this list of conditions and the following disclaimer. - * Redistributions in binary form must reproduce the above -copyright notice, this list of conditions and the following disclaimer -in the documentation and/or other materials provided with the -distribution. - * Neither the name of Google Inc. nor the names of its -contributors may be used to endorse or promote products derived from -this software without specific prior written permission. - -THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -*/ - -package main - -import ( - "testing" -) - -func TestFat(t *testing.T) { - tests := []struct { - file string - mht MachOType - arches []string - }{ - {"testdata/libarchtest32.dylib", MachODylib, []string{ArchI386}}, - {"testdata/libarchtest64.dylib", MachODylib, []string{ArchX86_64}}, - {"testdata/libarchtest.dylib", MachODylib, []string{ArchI386, ArchX86_64}}, - {"testdata/archtest32.exe", MachOExe, []string{ArchI386}}, - {"testdata/archtest64.exe", MachOExe, []string{ArchX86_64}}, - {"testdata/archtest.exe", MachOExe, []string{ArchI386, ArchX86_64}}, - } - - for _, e := range tests { - imageinfo, err := GetMachOImageInfo(e.file) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - - expected := make(map[string]bool) - for _, arch := range e.arches { - expected[arch] = false - } - - if len(imageinfo) != len(e.arches) { - t.Errorf("Wrong number of arches, got %d, expected %d", len(imageinfo), len(e.arches)) - } - - for _, ii := range imageinfo { - if ii.Type != e.mht { - t.Errorf("Wrong MachOType got %d, expected %d", ii.Type, e.mht) - } - if o, k := expected[ii.Arch]; o || !k { - t.Errorf("Unexpected architecture %q", ii.Arch) - } - expected[ii.Arch] = true - } - - for k, v := range expected { - if !v { - t.Errorf("Did not get expected architecture %s", k) - } - } - } -} diff --git a/src/tools/mac/upload_system_symbols/testdata/Makefile b/src/tools/mac/upload_system_symbols/testdata/Makefile deleted file mode 100644 index cb481a3c..00000000 --- a/src/tools/mac/upload_system_symbols/testdata/Makefile +++ /dev/null @@ -1,22 +0,0 @@ -all: libarchtest.dylib archtest.exe - -archtest32.exe: archtest.c - clang -m32 $< -o $@ - -archtest64.exe: archtest.c - clang -m64 $< -o $@ - -archtest.exe: archtest32.exe archtest64.exe - lipo $^ -create -output $@ - -libarchtest32.dylib: archtest.c - clang -m32 -dynamiclib $< -o $@ - -libarchtest64.dylib: archtest.c - clang -m64 -dynamiclib $< -o $@ - -libarchtest.dylib: libarchtest32.dylib libarchtest64.dylib - lipo $^ -create -output $@ - -clean: - rm -f *.dylib *.exe diff --git a/src/tools/mac/upload_system_symbols/testdata/archtest.c b/src/tools/mac/upload_system_symbols/testdata/archtest.c deleted file mode 100644 index 0931fca1..00000000 --- a/src/tools/mac/upload_system_symbols/testdata/archtest.c +++ /dev/null @@ -1,7 +0,0 @@ -int TestLibUsefulFunction() { - return 42; -} - -int main() { - return 0; -} diff --git a/src/tools/mac/upload_system_symbols/upload_system_symbols.go b/src/tools/mac/upload_system_symbols/upload_system_symbols.go index 869a2773..35561208 100644 --- a/src/tools/mac/upload_system_symbols/upload_system_symbols.go +++ b/src/tools/mac/upload_system_symbols/upload_system_symbols.go @@ -43,6 +43,7 @@ Both i386 and x86_64 architectures will be dumped and uploaded. package main import ( + "debug/macho" "flag" "fmt" "io" @@ -58,10 +59,11 @@ import ( ) var ( - breakpadTools = flag.String("breakpad-tools", "out/Release/", "Path to the Breakpad tools directory, containing dump_syms and symupload.") - uploadOnlyPath = flag.String("upload-from", "", "Upload a directory of symbol files that has been dumped independently.") - dumpOnlyPath = flag.String("dump-to", "", "Dump the symbols to the specified directory, but do not upload them.") - systemRoot = flag.String("system-root", "", "Path to the root of the Mac OS X system whose symbols will be dumped.") + breakpadTools = flag.String("breakpad-tools", "out/Release/", "Path to the Breakpad tools directory, containing dump_syms and symupload.") + uploadOnlyPath = flag.String("upload-from", "", "Upload a directory of symbol files that has been dumped independently.") + dumpOnlyPath = flag.String("dump-to", "", "Dump the symbols to the specified directory, but do not upload them.") + systemRoot = flag.String("system-root", "", "Path to the root of the Mac OS X system whose symbols will be dumped.") + dumpArchitecture = flag.String("arch", "", "The CPU architecture for which symbols should be dumped. If not specified, dumps all architectures.") ) var ( @@ -373,17 +375,46 @@ func (fq *findQueue) worker() { continue } - imageinfos, err := GetMachOImageInfo(fp) - if err != nil && err != ErrNotMachO { + f, err := os.Open(fp) + if err != nil { log.Printf("%s: %v", fp, err) continue } - for _, imageinfo := range imageinfos { - if imageinfo.Type == MachODylib || imageinfo.Type == MachOBundle { - - fq.dq.DumpSymbols(fp, imageinfo.Arch) + fatFile, err := macho.NewFatFile(f) + if err == nil { + // The file is fat, so dump its architectures. + for _, fatArch := range fatFile.Arches { + fq.dumpMachOFile(fp, fatArch.File) } + fatFile.Close() + } else if err == macho.ErrNotFat { + // The file isn't fat but may still be MachO. + thinFile, err := macho.NewFile(f) + if err != nil { + log.Printf("%s: %v", fp, err) + continue + } + fq.dumpMachOFile(fp, thinFile) + thinFile.Close() + } else { + f.Close() } } } + +func (fq *findQueue) dumpMachOFile(fp string, image *macho.File) { + if image.Type != MachODylib && image.Type != MachOBundle { + return + } + + arch := getArchStringFromHeader(image.FileHeader) + if arch == "" { + // Don't know about this architecture type. + return + } + + if (*dumpArchitecture != "" && *dumpArchitecture == arch) || *dumpArchitecture == "" { + fq.dq.DumpSymbols(fp, arch) + } +}