Closed Bug 1556136 Opened 4 months ago Closed 4 months ago

LeakSanitizer: [@ GetLine]

Categories

(Core :: JavaScript Engine, defect, P1, major)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox-esr60 --- wontfix
firefox67 --- wontfix
firefox68 --- fixed
firefox69 --- fixed

People

(Reporter: gkw, Assigned: jorendorff)

References

(Blocks 1 open bug)

Details

(Keywords: crash, jsbugmon, testcase, Whiteboard: [jsbugmon:update])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision a73077366144 (build with --enable-address-sanitizer, run with --fuzzing-safe --ion-offthread-compile=off --no-baseline --no-ion) with ASAN_OPTIONS=detect_leaks=1 in the environment variable with the left arrow in bash, e.g.:

ASAN_OPTIONS=detect_leaks=1 ./js-64-asan-linux-x86_64-a73077366144 --fuzzing-safe --ion-offthread-compile=off --no-baseline --no-ion < testcase.js

quit();

Backtrace:

=================================================================
==14673==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x55e7fd849c30 in __interceptor_strdup (/home/ubuntu/shell-cache/js-64-asan-linux-x86_64-a73077366144/js-64-asan-linux-x86_64-a73077366144+0x13a8c30)
#1 0x55e7fda9403b in readline /home/ubuntu/trees/mozilla-central/js/src/editline/editline.c:1000:17
#2 0x55e7fd9bcac4 in GetLine(_IO_FILE*, char const*) /home/ubuntu/trees/mozilla-central/js/src/shell/js.cpp:686:19
#3 0x55e7fd9bcac4 in ReadEvalPrintLoop(JSContext*, _IO_FILE*, bool) /home/ubuntu/trees/mozilla-central/js/src/shell/js.cpp:1362
#4 0x55e7fd9bcac4 in Process(JSContext*, char const*, bool, FileKind) /home/ubuntu/trees/mozilla-central/js/src/shell/js.cpp:1496
#5 0x55e7fd95f990 in main /home/ubuntu/trees/mozilla-central/js/src/shell/js.cpp
#6 0x7f8ff8b8bb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

SUMMARY: AddressSanitizer: 8 byte(s) leaked in 1 allocation(s).

Or just enter it in the shell, press Enter.

This seems to be an old issue. :jorendorff, how best to move forward here, is this a valid issue?

Flags: needinfo?(jorendorff)
Summary: LeakSanitizer: [@ GetLine] with evalInWorker → LeakSanitizer: [@ GetLine]
Attachment #9069414 - Attachment description: Bug 1556136 - Fix misuse of readline() leading to a minor memory leak in the JS shell. r?khyperia → Bug 1556136 - Fix misuse of readline() leading to a minor memory leak in the JS shell. r?jwalden

It's real. Easy fix, depending on how deep you want to go in C++ifying things.

Flags: needinfo?(jorendorff)

Jason, since you seem to be working on this I am going to assign it to you and give it a P1 priority.

Assignee: nobody → jorendorff
Status: NEW → ASSIGNED
Priority: -- → P1
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1f576eb69f81
Fix misuse of readline() leading to a minor memory leak in the JS shell. r=jwalden
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Comment on attachment 9069414 [details]
Bug 1556136 - Fix misuse of readline() leading to a minor memory leak in the JS shell. r?jwalden

Beta/Release Uplift Approval Request

  • User impact if declined: Makes life more difficult for our fuzzers
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a borderline NPOTB patch since it only affects the JS shell. I asked Gary and he confirmed that it would make his life easier for fuzzing if we were to backport this to Beta for 68. It grafts cleanly as-landed.
  • String changes made/needed:
Attachment #9069414 - Flags: approval-mozilla-beta?

Comment on attachment 9069414 [details]
Bug 1556136 - Fix misuse of readline() leading to a minor memory leak in the JS shell. r?jwalden

js shell fix, approved for 68.0b10

Attachment #9069414 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.