String subscript out of range

[andreadps]

I was just testing v0.8.8 and ran into a problem at lexer.cpp:35. On strip_ws(const std::string &s, unsigned margin), s can sometimes be an empty string, so size_t j = s.size() - 1 will be an overflow and an error will occur when the code tries to access s[j].

[sparkprime]

Thanks for the detailed report. I agree strip_ws should be changed. Do you have a test case that demonstrates this at the Jsonnet code level? I would like to determine how many people would be affected by this.

[andreadps]

Thanks for the fix.

I stumbled upon this when Jsonnet was going through std.jsonnet.h. There are empty lines in the license header.

[sparkprime]

Yeah that \n\n would definitely cause strip_ws to be called with a empty string, and that would have affected every execution of Jsonnet. Interestingly, this passes valgrind even with -O0. This seems to be because of the particular std::string implementation, i.e. s[j] == ' ' and friends was reading arbitrary memory inside the string object. I.e. j was essentially -1 so it was looking one byte left from the start of the char block. On my machine (64 bit ubuntu trusty gcc & clang), that caused no problem as the char to the left of the start of the char block was \0 so it immediately exited the loop and returned "". For you, either the std::string internal buffer was a separate malloc'd buffer, or maybe the buffer is nullptr (because it's an empty string) or maybe it walked far enough to walk off the end of the std::string.

What OS & compiler did you see this on?

thanks

[andreadps]

Sorry about the delay.

I am actually compiling it under Windows 10 with VS 2015. I would say this is not the kind of scenario you would like to support, but I have been using Jsonnet flawlessly for a few months for managing configuration, just like you would do with GCL for instance.

Thanks

[sparkprime]

Windows support sounds great. I tried it myself once but it crashed msvc (the old version I had around at that time). Probably this was due to incomplete c++11 support at that time. I'm glad to hear you can compile it now.

f:\vm.cpp(447): fatal error C1001: An internal error has occurred in the compiler.
  (compiler file 'msc1.cpp', line 1325)
   To work around this problem, try simplifying or changing the program near the locations listed above.
  Please choose the Technical Support command on the Visual C++ 
   Help menu, or open the Technical Support help file for more information
          f:\vm.cpp(502) : see reference to function template instantiation 'T *`anonymous-namespace'::Interpreter::makeHeap<`anonymous-namespace'::HeapArray,std::vector<`anonymous-namespace'::HeapThunk *,std::allocator<_Ty>>>(std::vector<_Ty,std::allocator<_Ty>>)' being compiled
          with
          [
              T=`anonymous-namespace'::HeapArray
  ,            _Ty=`anonymous-namespace'::HeapThunk *
          ]
  INTERNAL COMPILER ERROR in 'C:\Program Files\Microsoft Visual Studio 12.0\VC\bin\CL.exe'
      Please choose the Technical Support command on the Visual C++
      Help menu, or open the Technical Support help file for more information

The only problem is lack of automatic testing and additionally the lack of casual use Jsonnet gets in Windows. In particular I'm quite concerned with dos line endings and whether they work properly.