1328569 - [App Verifier] UnmapViewOfFile() called with null lpBaseAddress

Status: RESOLVED FIXED

(Core :: General, defect, P1)

Description

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: App verifier error report

The error is found when running one of the test suites with m-c revision c91249f41e37. App Verifier reports that mozilla::HandleToFilename() tries to call UnmapViewOfFile() with a memory block address with value = 0. Excerpt of the error log:

<avrf:logEntry Time="2017-01-03 : 14:16:26" LayerName="Memory" StopCode="0x60B" Severity="Error">
        <avrf:message>Trying to free virtual memory block that is already free.</avrf:message>
        <avrf:parameter1>0 - Memory block address.</avrf:parameter1>
        <avrf:parameter2>0 - Not used.</avrf:parameter2>
        <avrf:parameter3>0 - Not used.</avrf:parameter3>
        <avrf:parameter4>0 - Not used.</avrf:parameter4>
        <avrf:stackTrace>
                <avrf:trace>vfbasics!+7ff8af1b41d6 ( @ 0)</avrf:trace> 
                <avrf:trace>vfbasics!+7ff8af1b3f1f ( @ 0)</avrf:trace> 
                <avrf:trace>KERNELBASE!UnmapViewOfFile+22 ( @ 0)</avrf:trace> 
                <avrf:trace>vfbasics!+7ff8af1b4ab2 ( @ 0)</avrf:trace> 
                <avrf:trace>xul!mozilla::HandleToFilename+18e (e:\hg\mozilla-central\xpcom\io\fileutilswin.cpp @ 68)</avrf:trace>

Comment 1

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: Bug 1328569 - Don't call UnmapViewOfFile() with a null address.

Review commit: https://reviewboard.mozilla.org/r/102128/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/102128/

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8823586 [details]
Bug 1328569 - Don't call UnmapViewOfFile() with a null address.

https://reviewboard.mozilla.org/r/102128/#review102464

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/51419ce26c15
Don't call UnmapViewOfFile() with a null address. r=froydnj

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/51419ce26c15

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Should we consider uplifting this to 52 ahead of the next ESR as well?

Comment 6

[Cervantes Yu [:cyu] [:cervantes]]

It does no harm. We should uplift.

Comment 7

[Cervantes Yu [:cyu] [:cervantes]]

Comment on attachment 8823586 [details]
Bug 1328569 - Don't call UnmapViewOfFile() with a null address.

Approval Request Comment
[Feature/Bug causing the regression]: No bug. This is in the utility function for the Windows platform.
[User impact if declined]: Possibly stability issues.
[Is this code covered by automated tests?]: No. The impact of this bug is undefined behavior, though in reality it doesn't seem to have issues.
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No. This needs to be verified with debugger or dynamic check tool.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: This adds a simple check so that we don't call the system call with a null argument. It's obvious that this patch does no harm.
[String changes made/needed]: No.

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 8823586 [details]
Bug 1328569 - Don't call UnmapViewOfFile() with a null address.

silence static analyzer with a null check, beta52+

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ec7ea9c6e94a

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/ec7ea9c6e94a