diff --git a/core/org.eclipse.cdt.core/ChangeLog b/core/org.eclipse.cdt.core/ChangeLog index a66650cb0f2..53520649789 100644 --- a/core/org.eclipse.cdt.core/ChangeLog +++ b/core/org.eclipse.cdt.core/ChangeLog @@ -1,3 +1,10 @@ +2005-07-22 Chris Wiebe + Fix for PR 104605: MachO parsing + First pass at optimization. My test case went from >20 minutes to 5 seconds + * utils/org/eclipse/cdt/utils/macho/MachO.java + * utils/org/eclipse/cdt/utils/macho/MachOHelper.java + * utils/org/eclipse/cdt/utils/macho/parser/MachOBinaryObject.java + 2005-07-22 Chris Wiebe Fix for PR 104725 * model/org/eclipse/cdt/internal/core/model/BinaryElement.java diff --git a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/macho/MachO.java b/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/macho/MachO.java index c9dc4a5cecf..0a1d7e1fc6b 100644 --- a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/macho/MachO.java +++ b/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/macho/MachO.java @@ -8,16 +8,17 @@ * Contributors: * QNX Software Systems - Initial API and implementation * Craig Watson. + * Apple Computer - work on performance optimizations *******************************************************************************/ - package org.eclipse.cdt.utils.macho; - import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Comparator; -import java.util.Vector; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; import org.eclipse.cdt.core.CCorePlugin; import org.eclipse.cdt.utils.CPPFilt; @@ -40,8 +41,10 @@ public class MachO { private Section[] sections; /* sections from SegmentCommand */ SymtabCommand symtab; /* SymtabCommand that contains the symbol table */ - protected String EMPTY_STRING = ""; //$NON-NLS-1$ + protected static final String EMPTY_STRING = ""; //$NON-NLS-1$ + protected static final SymbolComparator symbol_comparator = new SymbolComparator(); + public class MachOhdr { @@ -572,19 +575,11 @@ public class MachO { } protected String string_from_macho_symtab(MachO.SymtabCommand symtab, int index) throws IOException { - StringBuffer str = new StringBuffer(); - byte tmp; if ( index > symtab.strsize ) { return EMPTY_STRING; } efile.seek(symtab.stroff + index); - while( true ) { - tmp = efile.readByte(); - if ( tmp == 0 ) - break; - str.append((char)tmp); - } - return str.toString(); + return getCStr(); } public class Symbol implements Comparable { @@ -713,18 +708,6 @@ public class MachO { return name; } - private Line getLine(long value) { - if (!debugsym) { - return null; - } - for (int l = 0; l < lines.length; l++) { - Line line = lines[l]; - if (value <= line.address) - return line; - } - return null; - } - /** * Returns line information in the form of filename:line * and if the information is not available may return null @@ -817,13 +800,33 @@ public class MachO { } } + private Line getLine(long value) { + if (!debugsym) { + return null; + } + + int ndx = Arrays.binarySearch(lines, new Long(value)); + if ( ndx >= 0 ) + return lines[ndx]; + if ( ndx == -1 ) { + return null; + } + ndx = -ndx - 1; + for (int l = ndx - 1; l < lines.length; l++) { + Line line = lines[l]; + if (value <= line.address) + return line; + } + return null; + } + /** * We have to implement a separate compararator since when we do the * binary search down below we are using a Long and a Symbol object * and the Long doesn't know how to compare against a Symbol so if * we compare Symbol vs Long it is ok, but not if we do Long vs Symbol. */ - class SymbolComparator implements Comparator { + public static class SymbolComparator implements Comparator { long val1, val2; public int compare(Object o1, Object o2) { @@ -850,7 +853,7 @@ public class MachO { /** * Simple class to implement a line table */ - public class Line implements Comparable { + public static class Line implements Comparable { public long address; public int lineno; public String file; @@ -1124,7 +1127,7 @@ public class MachO { } /* now create line table, sorted on address */ - ArrayList lineList = new ArrayList(nlines); + Map lineList = new HashMap(nlines); for (int s = 0; s < symbols.length; s++) { Symbol sym = symbols[s]; if (sym.n_type == Symbol.N_SLINE || sym.n_type == Symbol.N_FUN) { @@ -1132,15 +1135,16 @@ public class MachO { lentry.address = sym.n_value; lentry.lineno = sym.n_desc; - int l = lineList.indexOf(lentry); - if (l >= 0) - lentry = (Line)lineList.get(l); - else - lineList.add(lentry); - - if (sym.n_type == Symbol.N_FUN) { + Line lookup = (Line)lineList.get(lentry); + if (lookup != null) { + lentry = lookup; + } else { + lineList.put(lentry, lentry); + } + + if (lentry.function == null && sym.n_type == Symbol.N_FUN) { String func = sym.toString(); - if (func != null) { + if (func != null && func.length() > 0) { int colon = func.indexOf(':'); if (colon > 0) lentry.function = func.substring(0, colon); @@ -1152,19 +1156,17 @@ public class MachO { } } } - lineList.trimToSize(); - lines = (Line[])lineList.toArray(new Line[0]); + Set k = lineList.keySet(); + lines = (Line[]) k.toArray(new Line[k.size()]); Arrays.sort(lines); /* now check for file names */ for (int s = 0; s < symbols.length; s++) { Symbol sym = symbols[s]; if (sym.n_type == Symbol.N_SO) { - for (int l = 0; l < lines.length; l++) { - if (sym.n_value <= lines[l].address) { - lines[l].file = sym.toString(); - break; - } + Line line = getLine(sym.n_value); + if (line != null) { + line.file = sym.toString(); } } } @@ -1213,6 +1215,17 @@ public class MachO { return tlhints; } + private String getCStr() throws IOException { + StringBuffer str = new StringBuffer(); + while( true ) { + byte tmp = efile.readByte(); + if (tmp == 0) + break; + str.append((char)tmp); + } + return str.toString(); + } + private String getLCStr(int len) throws IOException { if (len == 0) return EMPTY_STRING; @@ -1523,15 +1536,14 @@ public class MachO { } public DyLib[] getDyLibs(int type) { - Vector v = new Vector(); - + ArrayList v = new ArrayList(); for (int i = 0; i < loadcommands.length; i++) { if (loadcommands[i].cmd == type) { DyLibCommand dl = (DyLibCommand)loadcommands[i]; v.add(dl.dylib); } } - return (DyLib[]) v.toArray(new DyLib[0]); + return (DyLib[]) v.toArray(new DyLib[v.size()]); } /* return the address of the function that address is in */ @@ -1540,9 +1552,6 @@ public class MachO { return null; } - //@@@ If this works, move it to a single instance in this class. - SymbolComparator symbol_comparator = new SymbolComparator(); - int ndx = Arrays.binarySearch(symbols, new Long(vma), symbol_comparator); if ( ndx > 0 ) return symbols[ndx]; diff --git a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/macho/MachOHelper.java b/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/macho/MachOHelper.java index b2b2fa327b7..5a1291909b1 100644 --- a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/macho/MachOHelper.java +++ b/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/macho/MachOHelper.java @@ -8,8 +8,8 @@ * Contributors: * QNX Software Systems - Initial API and implementation * Craig Watson + * Apple Computer - work on performance optimizations *******************************************************************************/ - package org.eclipse.cdt.utils.macho; import java.io.IOException; @@ -301,6 +301,8 @@ public class MachOHelper { data = 0; bss = 0; + // TODO further optimization + // TODO we only need to load the sections, not the whole shebang loadBinary(); for (int i = 0; i < sections.length; i++) { diff --git a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/macho/parser/MachOBinaryObject.java b/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/macho/parser/MachOBinaryObject.java index 12923c8aa9e..4c71005b11f 100644 --- a/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/macho/parser/MachOBinaryObject.java +++ b/core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/macho/parser/MachOBinaryObject.java @@ -6,17 +6,20 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * QNX Software Systems - Initial API and implementation + * QNX Software Systems - Initial API and implementation + * Apple Computer - work on performance optimizations *******************************************************************************/ package org.eclipse.cdt.utils.macho.parser; import java.io.ByteArrayInputStream; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.eclipse.cdt.core.IAddress; import org.eclipse.cdt.core.IAddressFactory; import org.eclipse.cdt.core.IBinaryParser; import org.eclipse.cdt.core.IBinaryParser.IBinaryFile; @@ -37,11 +40,17 @@ import org.eclipse.core.runtime.Path; */ public class MachOBinaryObject extends BinaryObjectAdapter { - private BinaryObjectInfo info; - private ISymbol[] symbols; - private AR.ARHeader header; - private IAddressFactory addressFactory; + protected AR.ARHeader header; + protected IAddressFactory addressFactory; + protected MachO.Attribute attributes; + protected MachOHelper.Sizes sizes; + protected ISymbol[] symbols; + protected String soname; + protected String[] needed; + protected long timeStamp; + private static final String[] NO_NEEDED = new String[0]; + /** * @param parser * @param path @@ -49,6 +58,7 @@ public class MachOBinaryObject extends BinaryObjectAdapter { */ public MachOBinaryObject(IBinaryParser parser, IPath path, AR.ARHeader header) { super(parser, path, IBinaryFile.OBJECT); + this.header = header; } /** @@ -60,36 +70,14 @@ public class MachOBinaryObject extends BinaryObjectAdapter { super(parser, path, type); } - /* (non-Javadoc) - * @see org.eclipse.cdt.core.IBinaryParser.IBinaryObject#getSymbols() - */ - public ISymbol[] getSymbols() { - // Call the hasChanged first, to initialize the timestamp - if (hasChanged() || symbols == null) { - try { - loadAll(); - } catch (IOException e) { - symbols = NO_SYMBOLS; - } - } - return symbols; - } - /* (non-Javadoc) * @see org.eclipse.cdt.utils.BinaryObjectAdapter#getBinaryObjectInfo() */ protected BinaryObjectInfo getBinaryObjectInfo() { - // Call the hasChanged first, to initialize the timestamp - if (hasChanged() || info == null) { - try { - loadInfo(); - } catch (IOException e) { - info = new BinaryObjectInfo(); - } - } - return info; + // we don't use this method + // overload to do nothing + return new BinaryObjectInfo(); } - /* (non-Javadoc) * @see org.eclipse.cdt.core.IBinaryParser.IBinaryFile#getContents() @@ -102,10 +90,15 @@ public class MachOBinaryObject extends BinaryObjectAdapter { } protected MachOHelper getMachOHelper() throws IOException { - if (header != null) { - return new MachOHelper(getPath().toOSString(), header.getObjectDataOffset()); + IPath path = getPath(); + if (path != null) { + if (header != null) { + return new MachOHelper(path.toOSString(), header.getObjectDataOffset()); + } else { + return new MachOHelper(path.toOSString()); + } } - return new MachOHelper(getPath().toOSString()); + return null; } /** @@ -118,12 +111,128 @@ public class MachOBinaryObject extends BinaryObjectAdapter { return super.getName(); } - protected void loadAll() throws IOException { + /* (non-Javadoc) + * @see org.eclipse.cdt.utils.BinaryObjectAdapter#getAddressFactory() + */ + public IAddressFactory getAddressFactory() { + if (addressFactory == null) { + addressFactory = new Addr32Factory(); + } + return addressFactory; + } + + protected void clearCachedValues() { + attributes = null; + sizes = null; + symbols = null; + soname = null; + needed = null; + } + + protected MachO.Attribute internalGetAttributes() { + if (hasChanged()) { + clearCachedValues(); + } + if (attributes == null) { + MachOHelper helper = null; + try { + helper = getMachOHelper(); + if (helper != null) { + attributes = helper.getMachO().getAttributes(); + } + } catch (IOException e) { + } finally { + if (helper != null) { + helper.dispose(); + } + } + } + return attributes; + } + + protected MachOHelper.Sizes internalGetSizes() { + if (hasChanged()) { + clearCachedValues(); + } + if (sizes == null) { + MachOHelper helper = null; + try { + helper = getMachOHelper(); + if (helper != null) { + sizes = helper.getSizes(); + // since we're invoking the helper we might as well update + // the attributes since it's a pretty lightweight operation + if (attributes == null) { + attributes = helper.getMachO().getAttributes(); + } + } + } catch (IOException e) { + } finally { + if (helper != null) { + helper.dispose(); + } + } + } + return sizes; + } + + protected ISymbol[] internalGetSymbols() { + if (hasChanged()) { + clearCachedValues(); + } + if (symbols == null) { + loadBinaryInfo(); + } + return symbols; + } + + protected String internalGetSoName() { + if (hasChanged()) { + clearCachedValues(); + } + if (soname == null) { + loadBinaryInfo(); + } + return soname; + } + + protected String[] internalGetNeeded() { + if (hasChanged()) { + clearCachedValues(); + } + if (needed == null) { + loadBinaryInfo(); + } + return needed; + } + + protected void loadBinaryInfo() { MachOHelper helper = null; try { helper = getMachOHelper(); - loadInfo(helper); - loadSymbols(helper); + if (helper != null) { + //TODO we can probably optimize this further in MachOHelper + + symbols = loadSymbols(helper); + //TODO is the sort necessary? + Arrays.sort(symbols); + + soname = helper.getSoname(); + needed = helper.getNeeded(); + + // since we're invoking the helper we might as well update the + // sizes since it's a pretty lightweight operation by comparison + if (sizes == null) { + sizes = helper.getSizes(); + } + // since we're invoking the helper we might as well update the + // attributes since it's a pretty lightweight operation by comparison + if (attributes == null) { + attributes = helper.getMachO().getAttributes(); + } + } + } catch (IOException e) { + symbols = NO_SYMBOLS; } finally { if (helper != null) { helper.dispose(); @@ -131,32 +240,24 @@ public class MachOBinaryObject extends BinaryObjectAdapter { } } - protected void loadInfo() throws IOException { - MachOHelper helper = null; + protected ISymbol[] loadSymbols(MachOHelper helper) throws IOException { + CPPFilt cppfilt = null; try { - helper = getMachOHelper(); - loadInfo(helper); + ArrayList list = new ArrayList(); + // Hack should be remove when Elf is clean + helper.getMachO().setCppFilter(false); + cppfilt = getCPPFilt(); + //TODO we can probably optimize this further in MachOHelper + addSymbols(helper.getExternalFunctions(), ISymbol.FUNCTION, cppfilt, list); + addSymbols(helper.getLocalFunctions(), ISymbol.FUNCTION, cppfilt, list); + addSymbols(helper.getExternalObjects(), ISymbol.VARIABLE, cppfilt, list); + addSymbols(helper.getLocalObjects(), ISymbol.VARIABLE, cppfilt, list); + return (ISymbol[]) list.toArray(new ISymbol[list.size()]); } finally { - if (helper != null) { - helper.dispose(); + if (cppfilt != null) { + cppfilt.dispose(); } - } - } - - protected void loadInfo(MachOHelper helper) throws IOException { - info = new BinaryObjectInfo(); - info.needed = helper.getNeeded(); - MachOHelper.Sizes sizes = helper.getSizes(); - info.bss = sizes.bss; - info.data = sizes.data; - info.text = sizes.text; - - info.soname = helper.getSoname(); - - MachO.Attribute attribute = helper.getMachO().getAttributes(); - info.isLittleEndian = attribute.isLittleEndian(); - info.hasDebug = attribute.hasDebug(); - info.cpu = attribute.getCPU(); + } } protected CPPFilt getCPPFilt() { @@ -164,29 +265,7 @@ public class MachOBinaryObject extends BinaryObjectAdapter { return parser.getCPPFilt(); } - protected void loadSymbols(MachOHelper helper) throws IOException { - ArrayList list = new ArrayList(); - // Hack should be remove when Elf is clean - helper.getMachO().setCppFilter(false); - - CPPFilt cppfilt = getCPPFilt(); - - addSymbols(helper.getExternalFunctions(), ISymbol.FUNCTION, cppfilt, list); - addSymbols(helper.getLocalFunctions(), ISymbol.FUNCTION, cppfilt, list); - addSymbols(helper.getExternalObjects(), ISymbol.VARIABLE, cppfilt, list); - addSymbols(helper.getLocalObjects(), ISymbol.VARIABLE, cppfilt, list); - list.trimToSize(); - - if (cppfilt != null) { - cppfilt.dispose(); - } - - symbols = (ISymbol[])list.toArray(NO_SYMBOLS); - Arrays.sort(symbols); - list.clear(); - } - - protected void addSymbols(MachO.Symbol[] array, int type, CPPFilt cppfilt, List list) { + private void addSymbols(MachO.Symbol[] array, int type, CPPFilt cppfilt, List list) { for (int i = 0; i < array.length; i++) { String name = array[i].toString(); if (cppfilt != null) { @@ -204,13 +283,96 @@ public class MachOBinaryObject extends BinaryObjectAdapter { } } - /* (non-Javadoc) - * @see org.eclipse.cdt.utils.BinaryObjectAdapter#getAddressFactory() - */ - public IAddressFactory getAddressFactory() { - if (addressFactory == null) { - addressFactory = new Addr32Factory(); + public String getCPU() { + MachO.Attribute attribute = internalGetAttributes(); + if (attribute != null) { + return attribute.getCPU(); } - return addressFactory; + return ""; //$NON-NLS-1$ + } + + public boolean hasDebug() { + MachO.Attribute attribute = internalGetAttributes(); + if (attribute != null) { + return attribute.hasDebug(); + } + return false; + } + + public boolean isLittleEndian() { + MachO.Attribute attribute = internalGetAttributes(); + if (attribute != null) { + return attribute.isLittleEndian(); + } + return false; + } + + public long getBSS() { + MachOHelper.Sizes size = internalGetSizes(); + if (size != null) { + return size.bss; + } + return 0; + } + + public long getData() { + MachOHelper.Sizes size = internalGetSizes(); + if (size != null) { + return size.data; + } + return 0; + } + + public long getText() { + MachOHelper.Sizes size = internalGetSizes(); + if (size != null) { + return size.text; + } + return 0; + } + + public ISymbol[] getSymbols() { + ISymbol[] syms = internalGetSymbols(); + if (syms != null) { + return syms; + } + return NO_SYMBOLS; + } + + public ISymbol getSymbol(IAddress addr) { + //TODO should this be cached? + // fall back to super implementation for now + return super.getSymbol(addr); + } + + public String[] getNeededSharedLibs() { + String[] libs = internalGetNeeded(); + if (libs != null) { + return libs; + } + return NO_NEEDED; + } + + public String getSoName() { + String name = internalGetSoName(); + if (name != null) { + return name; + } + return ""; //$NON-NLS-1$ + } + + protected boolean hasChanged() { + IPath path = getPath(); + if (path != null) { + File file = path.toFile(); + if (file != null) { + long modification = file.lastModified(); + if (modification != timeStamp) { + timeStamp = modification; + return true; + } + } + } + return false; } }