1556136 - LeakSanitizer: [@ GetLine]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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?

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1556136 - Fix misuse of readline() leading to a minor memory leak in the JS shell. r?jwalden

Comment 2

[Jason Orendorff [:jorendorff]]

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

Comment 3

[Steven DeTar [:sdetar]]

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

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f576eb69f81
Fix misuse of readline() leading to a minor memory leak in the JS shell. r=jwalden

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/1f576eb69f81

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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:

Comment 7

[Julien Cristau [:jcristau]]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/00a108b014da