Michael,
I have rework the patch according to your suggestion. And I have
read binutil/objdump source code and found that it has a logic that if
there's no symtab, it will use dynsym, which is missing in llvm-objdump.
Songmao
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Fix-the-address-calculation-for-llvm-objdump.patch
Type: text/x-patch
Size: 3036 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111012/8d8263d1/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Implement-sectionContainsSymbol-preparing-for-the-ob.patch
Type: text/x-patch
Size: 1321 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111012/8d8263d1/attachment-0001.bin>
On Wed, Oct 12, 2011 at 3:17 AM, Songmao <smtian at ingenic.cn> wrote:> Michael, > I have rework the patch according to your suggestion. And I have read > binutil/objdump source code and found that it has a logic that if there's no > symtab, it will use dynsym, which is missing in llvm-objdump. > > Songmao >@@ -747,12 +747,28 @@ error_code ELFObjectFile<target_endianness, is64Bits> template<support::endianness target_endianness, bool is64Bits> error_code ELFObjectFile<target_endianness, is64Bits> ::sectionContainsSymbol(DataRefImpl Sec, DataRefImpl Symb, bool &Result) const { - // FIXME: Unimplemented. + Result = false; + const Elf_Sym *sym = getSymbol(Symb); + const Elf_Shdr *sec = reinterpret_cast<const Elf_Shdr *>(Sec.p); + + if (getSection(sym->st_shndx) != sec) { This version of getSection will not handle extended section indices properly. Use the version that takes a const Elf_Sym*. This also fails when there is no section table, which is not required for executables or shared libraries. + Result = false; + return object_error::success; + } + + uint64_t Address; + getSymbolOffset(Symb, Address); + + if (sym->getType() == ELF::STT_FUNC Again, this will only work for functions. Symbols without this type can also be in a section. + && Address >= sec->sh_addr + && Address < (sec->sh_addr + sec->sh_size)) + Result = true; + return object_error::success; } template<support::endianness target_endianness, bool is64Bits> relocation_iterator ELFObjectFile<target_endianness, is64Bits> @@ -232,11 +235,11 @@ static void DisassembleObject(const ObjectFile *Obj) { if (error(i->getSize(SectSize))) break; // Disassemble symbol by symbol. for (unsigned si = 0, se = Symbols.size(); si != se; ++si) { uint64_t Start = Symbols[si].first; - uint64_t End = si == se-1 ? SectSize : Symbols[si + 1].first - 1; + uint64_t End = si == se-1 ? SectionVMA + SectSize : Symbols[si + 1].first - 1; This goes over 80 col. outs() << '\n' << Symbols[si].second << ":\n"; #ifndef NDEBUG raw_ostream &DebugOut = DebugFlag ? dbgs() : nulls(); #else @@ -244,22 +247,22 @@ static void DisassembleObject(const ObjectFile *Obj) { #endif for (Index = Start; Index < End; Index += Size) { MCInst Inst; - if (DisAsm->getInstruction(Inst, Size, memoryObject, Index, + outs() << format("%8x:\t", Index); + if (DisAsm->getInstruction(Inst, Size, memoryObject, Index - SectionVMA, DebugOut, nulls())) { - uint64_t addr; - if (error(i->getAddress(addr))) break; - outs() << format("%8x:\t", addr + Index); - DumpBytes(StringRef(Bytes.data() + Index, Size)); + DumpBytes(StringRef(Bytes.data() + Index - SectionVMA, Size)); IP->printInst(&Inst, outs(), ""); outs() << "\n"; } else { errs() << ToolName << ": warning: invalid instruction encoding\n"; if (Size == 0) - Size = 1; // skip illegible bytes + Size = 4; // skip illegible bytes This isn't right for most instruction sets. This value needs to be computed based on the current instruction set being disassembled. + DumpBytes(StringRef(Bytes.data() + Index - SectionVMA, Size)); + outs() << "\n"; } } } } } Once you fix these, I think it will be good to commit. Thanks for working on this! - Michael Spencer
Sorry for the late reply, please help question below> @@ -747,12 +747,28 @@ error_code ELFObjectFile<target_endianness, is64Bits> > template<support::endianness target_endianness, bool is64Bits> > error_code ELFObjectFile<target_endianness, is64Bits> > ::sectionContainsSymbol(DataRefImpl Sec, > DataRefImpl Symb, > bool&Result) const { > - // FIXME: Unimplemented. > + > Result = false; > + const Elf_Sym *sym = getSymbol(Symb); > + const Elf_Shdr *sec = reinterpret_cast<const Elf_Shdr *>(Sec.p); > + > + if (getSection(sym->st_shndx) != sec) { > > This version of getSection will not handle extended section indices > properly. Use the version that takes a const Elf_Sym*. This also fails > when there is no section table, which is not required for executables > or shared libraries.how can I make object file that has extended section indices, and that has no section table, so that I can verify the code> + Result = false; > + return object_error::success; > + } > + > + uint64_t Address; > + getSymbolOffset(Symb, Address); > + > + if (sym->getType() == ELF::STT_FUNC > > Again, this will only work for functions. Symbols without this type > can also be in a section.how to make some test cases?> + && Address>= sec->sh_addr > + && Address< (sec->sh_addr + sec->sh_size)) > + Result = true; > + > return object_error::success; > } > > template<support::endianness target_endianness, bool is64Bits> > relocation_iterator ELFObjectFile<target_endianness, is64Bits> > > @@ -232,11 +235,11 @@ static void DisassembleObject(const ObjectFile *Obj) { > if (error(i->getSize(SectSize))) break; > > // Disassemble symbol by symbol. > for (unsigned si = 0, se = Symbols.size(); si != se; ++si) { > uint64_t Start = Symbols[si].first; > - uint64_t End = si == se-1 ? SectSize : Symbols[si + 1].first - 1; > + uint64_t End = si == se-1 ? SectionVMA + SectSize : Symbols[si > + 1].first - 1; > > This goes over 80 col. > > outs()<< '\n'<< Symbols[si].second<< ":\n"; > > #ifndef NDEBUG > raw_ostream&DebugOut = DebugFlag ? dbgs() : nulls(); > #else > @@ -244,22 +247,22 @@ static void DisassembleObject(const ObjectFile *Obj) { > #endif > > for (Index = Start; Index< End; Index += Size) { > MCInst Inst; > > - if (DisAsm->getInstruction(Inst, Size, memoryObject, Index, > + outs()<< format("%8x:\t", Index); > + if (DisAsm->getInstruction(Inst, Size, memoryObject, Index - > SectionVMA, > DebugOut, nulls())) { > - uint64_t addr; > - if (error(i->getAddress(addr))) break; > - outs()<< format("%8x:\t", addr + Index); > - DumpBytes(StringRef(Bytes.data() + Index, Size)); > + DumpBytes(StringRef(Bytes.data() + Index - SectionVMA, Size)); > IP->printInst(&Inst, outs(), ""); > outs()<< "\n"; > } else { > errs()<< ToolName<< ": warning: invalid instruction encoding\n"; > if (Size == 0) > - Size = 1; // skip illegible bytes > + Size = 4; // skip illegible bytes > > This isn't right for most instruction sets. This value needs to be > computed based on the current instruction set being disassembled. >we can add a virtual method in the target disassembler class, so that the decision is made by the target code.> + DumpBytes(StringRef(Bytes.data() + Index - SectionVMA, Size)); > + outs()<< "\n"; > } > } > } > } > } > > Once you fix these, I think it will be good to commit. Thanks for > working on this! > > - Michael Spencer >Thank you for your help. Songmao