sstwcw added a comment. In the JavaScript tests that I added, it was wrong to use `SmallString`. Would you prefer me changing it to `string` or expanding the 6 test cases so we don't need a variable for the string?
Shortly after I committed this patch, the server builds caught the problem in the JavaScript tests. When the program reads a file, a null byte is added at the end. The program relies on this property because it means it doesn't have to do a bound check on the source string, including in some of the code I wrote though this time it was another piece of code that caught the problem. Also the null byte is not copied when a SmallString is initialized from a `const char*`. A null byte will be added if the `c_str` method is used at least once. But it was not the case here. So my test ended up passing a string which does not have a null byte at the end. I didn't catch the problem when I ran the tests myself because ironically I had the address sanitizer enabled. The address sanitizer didn't catch the problem because the byte past the end of the string in the `SmallString` is the struct padding which is still inside the object as far as the address sanitizer is concerned. However the address sanitizer likes to add padding between variables. So now the uninitialized part of the struct padding is the padding null bytes added by the address sanitizer instead of the variable that used to be there. So the assertion that the byte is null now holds. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154093/new/ https://reviews.llvm.org/D154093 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
