Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2)

Issue 6936058: code review 6936058: debug/elf: handle missing shstrndx in core files (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 6 months ago by dfc
Modified:
7 years, 1 month ago
Reviewers:
rsc, iant, dvyukov
CC:
minux1, rsc, golang-dev
Visibility:
Public.

Description

debug/elf: handle missing shstrndx in core files Fixes issue 4481. hello-world-core.gz was generated with a simple hello world c program and core dumped as suggested in the issue. Also: add support for gz compressed test fixtures.

Patch Set 1 #

Patch Set 2 : diff -r 5a40587c014a https://code.google.com/p/go #

Patch Set 3 : diff -r 7e135713451d https://code.google.com/p/go #

Patch Set 4 : diff -r 9e8d16052155 https://code.google.com/p/go #

Total comments: 1

Patch Set 5 : diff -r 9e8d16052155 https://code.google.com/p/go #

Patch Set 6 : diff -r 9e8d16052155 https://code.google.com/p/go #

Patch Set 7 : diff -r 9e8d16052155 https://code.google.com/p/go #

Patch Set 8 : diff -r 9e8d16052155 https://code.google.com/p/go #

Total comments: 2

Patch Set 9 : diff -r 9e8d16052155 https://code.google.com/p/go #

Total comments: 1

Patch Set 10 : diff -r 9a0e4777ea8b https://go.googlecode.com/hg/ #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -3 lines) Patch
M src/pkg/debug/elf/file.go View 1 2 3 4 5 6 7 8 2 chunks +6 lines, -1 line 1 comment Download
M src/pkg/debug/elf/file_test.go View 1 2 3 4 5 6 7 3 chunks +57 lines, -2 lines 0 comments Download
A src/pkg/debug/elf/testdata/hello-world-core.gz View 1 Binary file 0 comments Download

Messages

Total messages: 17
dfc
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
9 years, 6 months ago (2012-12-16 01:11:55 UTC) #1
minux1
do we really need to create a tmp file? i think just use bytes.Reader + ...
9 years, 6 months ago (2012-12-16 01:22:33 UTC) #2
dfc
Done. PTAL.
9 years, 6 months ago (2012-12-16 01:33:53 UTC) #3
minux1
https://codereview.appspot.com/6936058/diff/8001/src/pkg/debug/elf/file_test.go File src/pkg/debug/elf/file_test.go (right): https://codereview.appspot.com/6936058/diff/8001/src/pkg/debug/elf/file_test.go#newcode168 src/pkg/debug/elf/file_test.go:168: f, err := os.Open(tt.file) well i mean we keep ...
9 years, 6 months ago (2012-12-16 02:01:15 UTC) #4
dfc
> src/pkg/debug/elf/file_test.go:168: f, err := os.Open(tt.file) > well i mean we keep using elf.Open for ...
9 years, 6 months ago (2012-12-16 02:23:08 UTC) #5
dfc
Hello golang-dev@googlegroups.com, minux.ma@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 6 months ago (2012-12-16 02:25:05 UTC) #6
minux1
https://codereview.appspot.com/6936058/diff/4005/src/pkg/debug/elf/file.go File src/pkg/debug/elf/file.go (right): https://codereview.appspot.com/6936058/diff/4005/src/pkg/debug/elf/file.go#newcode275 src/pkg/debug/elf/file.go:275: if shstrndx < 0 || shstrndx > shnum { ...
9 years, 6 months ago (2012-12-16 02:32:02 UTC) #7
dfc
https://codereview.appspot.com/6936058/diff/4005/src/pkg/debug/elf/file.go File src/pkg/debug/elf/file.go (right): https://codereview.appspot.com/6936058/diff/4005/src/pkg/debug/elf/file.go#newcode275 src/pkg/debug/elf/file.go:275: if shstrndx < 0 || shstrndx > shnum { ...
9 years, 6 months ago (2012-12-16 04:56:59 UTC) #8
rsc
LGTM
9 years, 6 months ago (2012-12-17 14:51:32 UTC) #9
iant
LGTM https://codereview.appspot.com/6936058/diff/8006/src/pkg/debug/elf/file.go File src/pkg/debug/elf/file.go (right): https://codereview.appspot.com/6936058/diff/8006/src/pkg/debug/elf/file.go#newcode276 src/pkg/debug/elf/file.go:276: if shnum > 0 && shoff > 0 ...
9 years, 6 months ago (2012-12-17 20:50:36 UTC) #10
dfc
> For the record, a value of shstrndx == SHN_XINDEX (0xffff) is valid and means ...
9 years, 6 months ago (2012-12-17 20:56:44 UTC) #11
dfc
*** Submitted as https://code.google.com/p/go/source/detail?r=c2a84616b24e *** debug/elf: handle missing shstrndx in core files Fixes issue 4481. ...
9 years, 6 months ago (2012-12-17 20:59:27 UTC) #12
dvyukov
https://codereview.appspot.com/6936058/diff/8009/src/pkg/debug/elf/file.go File src/pkg/debug/elf/file.go (right): https://codereview.appspot.com/6936058/diff/8009/src/pkg/debug/elf/file.go#newcode276 src/pkg/debug/elf/file.go:276: if shnum > 0 && shoff > 0 && ...
7 years, 1 month ago (2015-05-29 18:41:47 UTC) #13
rsc
They are uint16s. They can't be negative.
7 years, 1 month ago (2015-05-29 20:15:32 UTC) #14
dvyukov
On 2015/05/29 20:15:32, rsc wrote: > They are uint16s. They can't be negative. Then I ...
7 years, 1 month ago (2015-05-29 21:16:38 UTC) #15
iant
On 2015/05/29 20:15:32, rsc wrote: > They are uint16s. They can't be negative. shnum is ...
7 years, 1 month ago (2015-05-29 21:32:27 UTC) #16
dvyukov
7 years, 1 month ago (2015-05-30 10:08:12 UTC) #17
Message was sent while issue was closed.
On 2015/05/29 21:32:27, iant wrote:
> On 2015/05/29 20:15:32, rsc wrote:
> > They are uint16s. They can't be negative.
> 
> shnum is a uint16.  shoff is an int64 representing a value stored in ELF as
> either uint32 or uint64; given a corrupt input file, it can be negative.
> 
> Note that this code doesn't handle valid ELF files with > 0xfff0 sections.

Filed https://github.com/golang/go/issues/10996 with standalone reproducer.
Let's move the discussion there.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b