code review 6551045: debug/pe: add symbol support

debug/pe: add symbol support

Add support for processing the COFF symbol table.

[brainman, 2012-09-21 04:52:01]

LGTM

Very nice. Thank you. Maybe add test:

diff -r 6a46db330eb9 src/pkg/debug/pe/file_test.go
--- a/src/pkg/debug/pe/file_test.go	Fri Sep 21 07:13:36 2012 +0900
+++ b/src/pkg/debug/pe/file_test.go	Fri Sep 21 14:49:48 2012 +1000
@@ -13,6 +13,7 @@
 	file     string
 	hdr      FileHeader
 	sections []*SectionHeader
+	symbols  []*Symbol
 }
 
 var fileTests = []fileTest{
@@ -33,6 +34,24 @@
 			{".debug_pubtypes", 0, 0, 38, 1370, 1580, 0, 1, 0, 1108344832},
 			{".debug_aranges", 0, 0, 32, 1408, 1590, 0, 2, 0, 1108344832},
 		},
+		[]*Symbol{
+			{".file", 0x0, -2, 0x0, 0x67},
+			{"_main", 0x0, 1, 0x20, 0x2},
+			{".text", 0x0, 1, 0x0, 0x3},
+			{".data", 0x0, 2, 0x0, 0x3},
+			{".bss", 0x0, 3, 0x0, 0x3},
+			{".debug_abbrev", 0x0, 4, 0x0, 0x3},
+			{".debug_info", 0x0, 5, 0x0, 0x3},
+			{".debug_line", 0x0, 6, 0x0, 0x3},
+			{".rdata", 0x0, 7, 0x0, 0x3},
+			{".debug_frame", 0x0, 8, 0x0, 0x3},
+			{".debug_loc", 0x0, 9, 0x0, 0x3},
+			{".debug_pubnames", 0x0, 10, 0x0, 0x3},
+			{".debug_pubtypes", 0x0, 11, 0x0, 0x3},
+			{".debug_aranges", 0x0, 12, 0x0, 0x3},
+			{"___main", 0x0, 0, 0x20, 0x2},
+			{"_puts", 0x0, 0, 0x20, 0x2},
+		},
 	},
 	{
 		"testdata/gcc-386-mingw-exec",
@@ -54,6 +73,7 @@
 			{Name: ".debug_frame", VirtualSize: 0x34, VirtualAddress: 0xe000, Size:
0x200, Offset: 0x3800, PointerToRelocations: 0x0, PointerToLineNumbers: 0x0,
NumberOfRelocations: 0x0, NumberOfLineNumbers: 0x0, Characteristics:
0x42300000},
 			{Name: ".debug_loc", VirtualSize: 0x38, VirtualAddress: 0xf000, Size: 0x200,
Offset: 0x3a00, PointerToRelocations: 0x0, PointerToLineNumbers: 0x0,
NumberOfRelocations: 0x0, NumberOfLineNumbers: 0x0, Characteristics:
0x42100000},
 		},
+		[]*Symbol{},
 	},
 }
 
@@ -86,7 +106,15 @@
 		if tn != fn {
 			t.Errorf("open %s: len(Sections) = %d, want %d", tt.file, fn, tn)
 		}
-
+		for i, have := range f.Symbols {
+			if i >= len(tt.symbols) {
+				break
+			}
+			want := tt.symbols[i]
+			if !reflect.DeepEqual(have, want) {
+				t.Errorf("open %s, symbol %d:\n\thave %#v\n\twant %#v\n", tt.file, i, have,
want)
+			}
+		}
 	}
 }

https://codereview.appspot.com/6551045/diff/1001/src/pkg/debug/pe/file.go
File src/pkg/debug/pe/file.go (right):

https://codereview.appspot.com/6551045/diff/1001/src/pkg/debug/pe/file.go#new...
src/pkg/debug/pe/file.go:179: name = cstring(cs.Name[0:])
s/0//

[jsing, 2012-09-21 17:27:32]

Hello alex.brainman@gmail.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[jsing, 2012-09-21 17:30:28]

On 2012/09/21 04:52:01, brainman wrote:
> LGTM
> 
> Very nice. Thank you. Maybe add test:

I had planned on adding a test - thanks for providing one :)

As an aside, I note that the debug/macho and debug/elf packages do not have
symbol related tests.

[jsing, 2012-09-22 07:57:33]

*** Submitted as http://code.google.com/p/go/source/detail?r=f56656ba8aac ***

debug/pe: add symbol support

Add support for processing the COFF symbol table.

R=alex.brainman
CC=golang-dev
http://codereview.appspot.com/6551045

[r, 2012-09-22 08:12:15]

http://codereview.appspot.com/6551045/diff/3003/src/pkg/debug/pe/file.go
File src/pkg/debug/pe/file.go (right):

http://codereview.appspot.com/6551045/diff/3003/src/pkg/debug/pe/file.go#newc...
src/pkg/debug/pe/file.go:176: si := int(binary.LittleEndian.Uint32(cs.Name[4:]))
is PE defined to be little-endian?

[brainman, 2012-09-22 08:34:38]

On 2012/09/22 08:12:15, r wrote:
> is PE defined to be little-endian?

Yes. For example, look for Endian on http://www.thehackerslibrary.com/?p=377.

Alex