Bug 190228 - [JSC] print() changes CRLF to CRCRLF on Windows
Summary: [JSC] print() changes CRLF to CRCRLF on Windows
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Windows 10
: P2 Normal
Assignee: Ross Kirsling
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-02 17:02 PDT by Ross Kirsling
Modified: 2018-10-03 22:13 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.47 KB, patch)
2018-10-03 15:34 PDT, Ross Kirsling
no flags Details | Formatted Diff | Diff
Patch (1.66 KB, patch)
2018-10-03 15:55 PDT, Ross Kirsling
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ross Kirsling 2018-10-02 17:02:03 PDT
Repro:
1. Create test.js:
> print(function foo() {
> });

2. Run it (with WinCairo JSC) and view output with explicit carriage returns:
> jsc.exe test.js | cat -v

3. Notice that carriage returns inside the function are doubled:
> function fn() {^M^M
> }^M

---

Notes:
- This results in various test failures under the ChakraCore/test directory:
    Basics/with3.js.default
    Closures/closure-funcexpr-eval.js.default
    Function/CallerArgs.js.default
    Function/funcExpr.js.default
    Function/moreFuncExpr.js.default
    Function/stackargs.js.default
    Function/toString.js.default
    LetConst/l.js.default
    strict/05.arguments.js.default
- It appears that read/readFile also produce CRCRLF in function bodies; perhaps this is why there aren't even more failures (e.g. stress/jsc-read.js).
Comment 1 Ross Kirsling 2018-10-03 15:34:30 PDT
Created attachment 351553 [details]
Patch
Comment 2 Mark Lam 2018-10-03 15:45:02 PDT
Comment on attachment 351553 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=351553&action=review

> Source/JavaScriptCore/jsc.cpp:2236
> +    _setmode(1, _O_BINARY);
> +    _setmode(2, _O_BINARY);

Would it be possible to use _fileno(stdout) and _fileno(stderr) instead?  I think that those documents the intent better than 1 and 2.
Comment 3 Ross Kirsling 2018-10-03 15:55:02 PDT
Created attachment 351555 [details]
Patch
Comment 4 Mark Lam 2018-10-03 15:56:19 PDT
Comment on attachment 351555 [details]
Patch

r=me if the EWS bots are green.
Comment 5 WebKit Commit Bot 2018-10-03 22:12:27 PDT
Comment on attachment 351555 [details]
Patch

Clearing flags on attachment: 351555

Committed r236827: <https://trac.webkit.org/changeset/236827>
Comment 6 WebKit Commit Bot 2018-10-03 22:12:29 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2018-10-03 22:13:23 PDT
<rdar://problem/44998734>