All users were logged out of Bugzilla on October 13th, 2018

memory corruption - xpt_link fails when asked to link a large .xpt file

RESOLVED FIXED in M10

Status

()

P3
major
RESOLVED FIXED
20 years ago
10 years ago

People

(Reporter: mike+mozilla, Assigned: shaver)

Tracking

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

20 years ago
When I do 'xpt_link outfile *.xpt' in the components directory (to see just how
small it can be made) xpt_link fails with a seg fault.  Looks like it's memory
corruption; realloc in libxpt (or malloc too, tried that) fails when trying to
grow the data pool from 8k to 16k.

This could be limited to xpt_link, or it could be a problem with libxpt, which
means that it could affect xpidl and possibly (less likely) the xptinfo runtime
as well.

Updated

20 years ago
Assignee: mccabe → mang

Comment 1

20 years ago
Reassigning to self/easing mccabe's pain.
(Reporter)

Comment 2

20 years ago
A little bird told me that there may be tools available to debug this sort of
problem on win32.  Would Purify find it?

Comment 3

20 years ago
Created attachment 883 [details]
purify trace

Comment 4

20 years ago
Looking at the purify trace, it seems that pool->data gets set beyond the end of
the buffer, eventually causing the crash in Grow_Pool.

Updated

20 years ago
Status: NEW → ASSIGNED
Target Milestone: M10

Comment 5

19 years ago
Mass reassign to mccabe since I'm outta here.
Assignee: mccabe → shaver
I have a tentative fix, which I've mailed to mccabe for review.

Basically, we were only growing the data pool when we were writing in the _data_
section, so if you had a header that was > XPT_ALLOC_CHUNK you'd fault out.  I
have a patch that fixes that, makes DoHeader's annotation processing interative
rather than recursive to avoid blowing the stack in the link-many-typelibs case.

I can now link components/*.xpt into one mozilla.xpt, which should also help our
startup performance.

I _don't_ see the realloc failure that mang mentions, though; we never try to
realloc in this failure mode, as far as I can tell.  So it's possible that
there's another bug lurking.

(Oh, and I killed a compile warning, too. =) )

mccabe: if you positively review my diff, just mark this fixed.

Comment 7

19 years ago
shaver, if you have not alreay checked in your patch by the timne you read
this could you sent it to me?

I'm playing with linking all these files and seeing a bad assert. When I run
under purify I see uninitialized memreads, array out of bounds writes and the
like.

Even just linking 2 xpt files I see one case of an uninitialized memory read
when doing a XPT_Do32 "u.b8[3] = CURS_POINT(cursor);" doing ENCODE even though
it has clearly done a CHECK_CURSOR. I'm thinking that the buffer size is getting
out of whack with the info about the buffer size somewhere. Even though we are
not always stepping on memory in use for something else, we are getting out of
our sandbox sometimes even on small links. Is this what you fixed?

I'll give it a try in purify when it comes down the pike.

John.

Comment 8

19 years ago
Shaver's patch makes it not assert for me. I'm still seeing some purify oddness.
I'm not yet convinced that this is not a result of the idl array support junk
added to libxpt in my tree. I'll work on that problem.
(Reporter)

Comment 9

19 years ago
Shaver - your diffs look fine to me.

Is blowing the stack really a problem?

Also, I believe malloc was failing b/c one of its structures was corrupted - as
the specific manifestation of memory corruption is usually platform specific, I
wouldn't worry if you don't see that problem.
I think the stack blowing is a real concern; I was seeing it when linking large
sets of files (*.xpt -> mozilla.xpt).  Is the iterative nature of my fix
particularily repellent?
(Reporter)

Comment 11

19 years ago
Not at all, especially if you were noticing stack blowage.  I'm just not used to
seeing stack as a constrained resource.  Did DoAnnotation push a lot?
I don't think it was extreme in its per-frame usage, but coop and I never took
pains to minimize (or even analyze) it.  When linking *.xpt, though, you can
have some 300 annotations to emit, which causes a pretty deep call chain.
Status: NEW → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED

Updated

10 years ago
Component: xpidl → XPCOM
QA Contact: mike+mozilla → xpcom
You need to log in before you can comment on or make changes to this bug.