buffer: Prevent Buffer constructor deopt

[brycebaril]

The Buffer constructor will generally get inlined, but any call to the Buffer
constructor for a string without encoding will cause an eager deoptimization
of any function that inlined the Buffer constructor. This is due to a an
out-of-bounds read on arguments[1]. This change prevents that deopt.

This example script demonstrates the deoptimization:

function main() {
  function makeBuffer() {
    var b = new Buffer(10).fill(0);
  }

  for (var i = 0; i < 1e5; i++) {
    makeBuffer()
  }

  b = new Buffer('abcd');

  makeBuffer();
}

main();

If run with --trace-deopt:

$ ./node --trace-deopt t.js
[deoptimizing (DEOPT eager): begin 0x850afb4d981 <JS Function main (SharedFunctionInfo 0x850afb2ecb1)> (opt #8) @18, FP to SP delta: 152]
            ;;; deoptimize at 1156: out of bounds
  reading input frame main => node=1, args=68, height=3; inputs:
      0: 0x850afb4d981 ; (frame function) 0x850afb4d981 <JS Function main (SharedFunctionInfo 0x850afb2ecb1)>
      1: 0x257b60004189 ; [fp - 72] 0x257b60004189 <undefined>
      2: 0x257b600b62d1 ; [fp - 64] 0x257b600b62d1 <FixedArray[164]>
      3: 0x850afb4d939 ; [fp - 56] 0x850afb4d939 <JS Function makeBuffer (SharedFunctionInfo 0x850afb2f659)>
      4: 0x257b60004189 ; (literal 4) 0x257b60004189 <undefined>
  reading construct stub frame main => height=2; inputs:
      0: 0x31834c93da41 ; (literal 8) 0x31834c93da41 <JS Function Buffer (SharedFunctionInfo 0x257b600f50d9)>
      1: 0x28bb10dd7b1 ; rbx 0x28bb10dd7b1 <a Buffer with map 0x1ac5dac178c9>
      2: 0x850afb2ec31 ; (literal 13) 0x850afb2ec31 <String[4]: abcd>
  reading input frame Buffer => node=2, args=74, height=2; inputs:
      0: 0x31834c93da41 ; (literal 8) 0x31834c93da41 <JS Function Buffer (SharedFunctionInfo 0x257b600f50d9)>
      1: 0x28bb10dd7b1 ; rbx 0x28bb10dd7b1 <a Buffer with map 0x1ac5dac178c9>
      2: 0x850afb2ec31 ; (literal 13) 0x850afb2ec31 <String[4]: abcd>
      3: 0x31834c93cea9 ; (literal 10) 0x31834c93cea9 <FixedArray[26]>
      4: argumets object #0 (length = 1)
           0x850afb2ec31 ; (literal 13) 0x850afb2ec31 <String[4]: abcd>
  translating frame main => node=68, height=16
    0x7ffe14593378: [top + 48] <- 0x257b60004189 ;  0x257b60004189 <undefined>  (input #1)
    0x7ffe14593370: [top + 40] <- 0x3fdb34b4ce86 ;  caller's pc
    0x7ffe14593368: [top + 32] <- 0x7ffe145933a0 ;  caller's fp
    0x7ffe14593360: [top + 24] <- 0x257b600b62d1 ;  context    0x257b600b62d1 <FixedArray[164]>  (input #2)
    0x7ffe14593358: [top + 16] <- 0x850afb4d981 ;  function    0x850afb4d981 <JS Function main (SharedFunctionInfo 0x850afb2ecb1)>  (input #0)
    0x7ffe14593350: [top + 8] <- 0x850afb4d939 ;  0x850afb4d939 <JS Function makeBuffer (SharedFunctionInfo 0x850afb2f659)>  (input #3)
    0x7ffe14593348: [top + 0] <- 0x257b60004189 ;  0x257b60004189 <undefined>  (input #4)
  translating construct stub => height=16
    0x7ffe14593340: [top + 80] <- 0x28bb10dd7b1 ;  0x28bb10dd7b1 <a Buffer with map 0x1ac5dac178c9>  (input #1)
    0x7ffe14593338: [top + 72] <- 0x850afb2ec31 ;  0x850afb2ec31 <String[4]: abcd>  (input #2)
    0x7ffe14593330: [top + 64] <- 0x3fdb34b4d4ed ;  caller's pc
    0x7ffe14593328: [top + 56] <- 0x7ffe14593368 ;  caller's fp
    0x7ffe14593320: [top + 48] <- 0x257b600b62d1 ;  context
    0x7ffe14593318: [top + 40] <- 0x900000000 ;  function (construct sentinel)
    0x7ffe14593310: [top + 32] <- 0x3fdb34a2eea1 ;  code object
    0x7ffe14593308: [top + 24] <- 0x257b60004189 ;  allocation site
    0x7ffe14593300: [top + 16] <- 0x100000000 ;  argc (1)
    0x7ffe145932f8: [top + 8] <- 0x257b60004189 ;  new.target
    0x7ffe145932f0: [top + 0] <- 0x28bb10dd7b1 ;  allocated receiver
  translating frame Buffer => node=74, height=8
    0x7ffe145932e8: [top + 48] <- 0x28bb10dd7b1 ;  0x28bb10dd7b1 <a Buffer with map 0x1ac5dac178c9>  (input #1)
    0x7ffe145932e0: [top + 40] <- 0x850afb2ec31 ;  0x850afb2ec31 <String[4]: abcd>  (input #2)
    0x7ffe145932d8: [top + 32] <- 0x3fdb34a2f0a6 ;  caller's pc
    0x7ffe145932d0: [top + 24] <- 0x7ffe14593328 ;  caller's fp
    0x7ffe145932c8: [top + 16] <- 0x31834c93cea9 ;  context    0x31834c93cea9 <FixedArray[26]>  (input #3)
    0x7ffe145932c0: [top + 8] <- 0x31834c93da41 ;  function    0x31834c93da41 <JS Function Buffer (SharedFunctionInfo 0x257b600f50d9)>  (input #0)
    0x7ffe145932b8: [top + 0] <- 0x257b600043d1 ;  0x257b600043d1 <Odd Oddball>  (input #4)
[deoptimizing (eager): end 0x850afb4d981 <JS Function main (SharedFunctionInfo 0x850afb2ecb1)> @18 => node=74, pc=0x3fdb34b48b68, state=NO_REGISTERS, alignment=no padding, took 0.089 ms]
Materialization [0x7ffe145932b8] <- 0x28bb10dd7c9 ;  0x28bb10dd7c9 <an Arguments with map 0x36dcdfa0b271>
[removing optimized code for: main]

Here is the output of IRHydra showing the deopts:

We can see that the Buffer constructor has been inlined:

The inlined Buffer constructor has been deoptimized due to arguments[1] out-of-bounds read:

After this patch, the Buffer constructor no longer will get deoptimized for out-of-bounds arguments reads:

./node --trace-deopt t.js

(no output)

cc: @trevnorris

[cjihrig]

LGTM

[jasnell]

LGTM. Good catch.

[trevnorris]

LGTM. Obligatory CI: https://ci.nodejs.org/job/node-test-pull-request/926/

[JungMinu]

LGTM as CI is happy :)
(one test result is unstable, but it seems unrelated)
May I land this?

[jasnell]

+1 go for it @JungMinu

[JungMinu]

Thanks, landed in 7239494

[ronkorving]

Wow, does passing arguments[n] really deopt? That's amazing. We have a lot of these cases, in order of level of concern (imho) in:

• events.js
• timers.js
• net.js
• dgram.js
• fs.js
• child_process.js
• util.js

Those should all be addressed then, no?

[evanlucas]

only if n >= arguments.length

[ronkorving]

Aaah, understood. Thank you. No cause for alarm then I guess.