esp32_https_server icon indicating copy to clipboard operation
esp32_https_server copied to clipboard

Array instantiation from non const variable or define

Open thebaron06 opened this issue 5 years ago • 1 comments

Describe the bug The char array defined here should be created from a non const variable or a define. IIRC this is not valid C/C++ code and the behavior is undefined. It is very likely that other data is overwritten. I am wondering that this code compiles at all.

How To Reproduce Call intToString.

Software (please complete the following information if applicable)

  • IDE and Version: all
  • OS: all

Possible solutions Use a string as buffer and resize to the required length because the function returns a string anyway.

thebaron06 avatar Jan 29 '20 17:01 thebaron06

It would've been valid C code, but for C++ (and that's what this source file is), it is not. Thank you for pointing that out.

However, the current xtensa toolchain (5.2.0 for my environment) treats this as expected, even though that's nothing to rely on:

00000000 <_ZN11httpsserver11intToStringB5cxx11Ei>:
   0:   004136          entry   a1, 32
   3:   017d            mov.n   a7, a1

bnez.n: If a3 (parameter i) isn't 0, go to 0x14 ->following block is if (i==0) return "0";
   5:   b3cc            bnez.n  a3, 14 <_ZN11httpsserver11intToStringB5cxx11Ei+0x14>
   7:   328b            addi.n  a3, a2, 8
   9:   0239            s32i.n  a3, a2, 0
   b:   0000c1          l32r    a12, fffc000c <.LC7+0xfffbffe2>
   e:   0000b1          l32r    a11, fffc0010 <.LC7+0xfffbffe6>
  11:   001ec6          j       90 <_ZN11httpsserver11intToStringB5cxx11Ei+0x90>

add 1 to i and store it in a10
  14:   a31b            addi.n  a10, a3, 1

call log10 and ceil, result will be in a10 after
  16:   000081          l32r    a8, fffc0018 <.LC7+0xfffbffee>
  19:   0008e0          callx8  a8
  1c:   000081          l32r    a8, fffc001c <.LC7+0xfffbfff2>
  1f:   0008e0          callx8  a8
  22:   000081          l32r    a8, fffc0024 <.LC7+0xfffbfffa>
  25:   0008e0          callx8  a8
  28:   000081          l32r    a8, fffc0028 <.LC7+0xfffbfffe>
  2b:   0008e0          callx8  a8

add 16 to the result and ...
  2e:   10ca42          addi    a4, a10, 16

... align it to multiples of 16
  31:   414440          srli    a4, a4, 4
  34:   1144c0          slli    a4, a4, 4

subtract the array size in a4 from the current stack pointer in a1, giving a4
  37:   c04140          sub     a4, a1, a4

actually move the stack pointer to create space for the new char array
  3a:   001410          movsp   a1, a4

This should give us space for the array which is at least as long as required for the content (provided that I read that assembly correctly).

However, most of the code in util.cpp can be way more cpp-ish. There shouldn't be a need for doing these conversions by hand at all. So replacing these custom functions is more likely what I would do to solve this issue. I'll keep it for the next release, but that may take some weeks.

fhessel avatar Jan 29 '20 20:01 fhessel