Parse source map in BCProviderFromSrc::create

Summary:
We currently fail to link `hermesvmlean` because we take a dependency on
the `SourceMapParser` which in turn requires the `JSONParser`. We
currently only use it to pass a parsed `SourceMap` to
`createBCProviderFromSrc`, so instead pass the actual source map and
perform the parsing there. This allows us to compile out the
`SourceMapParser` in lean builds.

While we're changing the signatures, clarify and enforce the null
termination requirement for the source map.

Reviewed By: avp

Differential Revision: D68936292

fbshipit-source-id: c1d00630c85c7cdb4a461100abd62cc7aa41f3bf

Author: neildhar

API/hermes/CompileJS.cpp

@@ -54,14 +54,16 @@ bool compileJS(
   flags.emitAsyncBreakCheck = compileJSOptions.emitAsyncBreakCheck;
   flags.inlineMaxSize = compileJSOptions.inlineMaxSize;
 
-  std::unique_ptr<hermes::SourceMap> sourceMap{};
-  // parse the source map if one was provided
+  // If there is a source map, ensure that it is null terminated, copying it if
+  // needed.
+  std::string smCopy;
+  llvh::StringRef smRef;
   if (sourceMapBuf.has_value()) {
-    hermes::SourceErrorManager sm;
-    sourceMap = hermes::SourceMapParser::parse(
-        llvh::StringRef{sourceMapBuf->data(), sourceMapBuf->size()}, {}, sm);
-    if (!sourceMap) {
-      return false;
+    if (sourceMapBuf->back() != '\0') {
+      smCopy = *sourceMapBuf;
+      smRef = {smCopy.data(), smCopy.size() + 1};
+    } else {
+      smRef = {sourceMapBuf->data(), sourceMapBuf->size()};
     }
   }
 
@@ -70,7 +72,7 @@ bool compileJS(
   auto res = hbc::createBCProviderFromSrc(
       std::make_unique<hermes::Buffer>((const uint8_t *)str.data(), str.size()),
       sourceURL,
-      std::move(sourceMap),
+      smRef,
       flags,
       "global",
       diagHandler ? diagHandlerAdapter : nullptr,

API/hermes/hermes.cpp

@@ -1536,27 +1536,15 @@ HermesRuntimeImpl::prepareJavaScriptWithSourceMap(
         std::make_unique<BufferAdapter>(jsiBuffer));
     runtimeFlags.persistent = true;
   } else {
-    std::unique_ptr<::hermes::SourceMap> sourceMap{};
+    llvh::StringRef sourceMap;
+    std::unique_ptr<BufferAdapter> buf0;
     if (sourceMapBuf) {
-      auto buf0 = ensureZeroTerminated(sourceMapBuf);
-      // Convert the buffer into a form the parser needs.
-      llvh::MemoryBufferRef mbref(
-          llvh::StringRef((const char *)buf0->data(), buf0->size()), "");
-      ::hermes::SimpleDiagHandler diag;
-      ::hermes::SourceErrorManager sm;
-      diag.installInto(sm);
-      sourceMap = ::hermes::SourceMapParser::parse(mbref, {}, sm);
-      if (!sourceMap) {
-        auto errorStr = diag.getErrorString();
-        LOG_EXCEPTION_CAUSE("Error parsing source map: %s", errorStr.c_str());
-        throw std::runtime_error("Error parsing source map:" + errorStr);
-      }
+      buf0 = ensureZeroTerminated(sourceMapBuf);
+      // The source map StringRef must include the null terminator.
+      sourceMap = llvh::StringRef((const char *)buf0->data(), buf0->size() + 1);
     }
     bcErr = hbc::createBCProviderFromSrc(
-        ensureZeroTerminated(jsiBuffer),
-        sourceURL,
-        std::move(sourceMap),
-        compileFlags_);
+        ensureZeroTerminated(jsiBuffer), sourceURL, sourceMap, compileFlags_);
     if (bcErr.first) {
       runtimeFlags.persistent = bcErr.first->allowPersistent();
     }

include/hermes/BCGen/HBC/BCProviderFromSrc.h

@@ -87,7 +87,9 @@ class BCProviderFromSrc final : public BCProviderBase {
   ///   "eval" for eval, "global" for ordinary scripts.
   /// \param sourceURL this will be used as the "file name" of the buffer for
   ///     errors, stack traces, etc.
-  /// \param sourceMap optional input source map for \p buffer.
+  /// \param sourceMap optional input source map for \p buffer. It may be empty
+  ///     if no source map is provided. If it is non-empty, the last byte must
+  ///     be a null terminator.
   /// \param compileFlags self explanatory
   /// \param diagHandler handler for errors/warnings/notes.
   /// \param diagContext opaque data that will be passed to diagHandler.
@@ -104,7 +106,7 @@ class BCProviderFromSrc final : public BCProviderBase {
   static std::pair<std::unique_ptr<BCProviderFromSrc>, std::string> create(
       std::unique_ptr<Buffer> buffer,
       llvh::StringRef sourceURL,
-      std::unique_ptr<SourceMap> sourceMap,
+      llvh::StringRef sourceMap,
       const CompileFlags &compileFlags,
       llvh::StringRef topLevelFunctionName = "global",
       SourceErrorManager::DiagHandlerTy diagHandler = {},

include/hermes/BCGen/HBC/HBC.h

@@ -87,7 +87,9 @@ void fullOptimizationPipeline(Module &M);
 ///   "eval" for eval, "global" for ordinary scripts.
 /// \param sourceURL this will be used as the "file name" of the buffer for
 ///     errors, stack traces, etc.
-/// \param sourceMap optional input source map for \p buffer.
+/// \param sourceMap optional input source map for \p buffer. It may be empty
+///     if no source map is provided. If it is non-empty, the last byte must
+///     be a null terminator.
 /// \param compileFlags self explanatory
 /// \param diagHandler handler for errors/warnings/notes.
 /// \param diagContext opaque data that will be passed to diagHandler.
@@ -104,7 +106,7 @@ void fullOptimizationPipeline(Module &M);
 std::pair<std::unique_ptr<BCProvider>, std::string> createBCProviderFromSrc(
     std::unique_ptr<Buffer> buffer,
     llvh::StringRef sourceURL,
-    std::unique_ptr<SourceMap> sourceMap,
+    llvh::StringRef sourceMap,
     const CompileFlags &compileFlags,
     llvh::StringRef topLevelFunctionName = "global",
     SourceErrorManager::DiagHandlerTy diagHandler = {},

lib/BCGen/HBC/BCProviderFromSrc.cpp

@@ -14,6 +14,7 @@
 #include "hermes/Runtime/Libhermes.h"
 #include "hermes/Sema/SemContext.h"
 #include "hermes/Sema/SemResolve.h"
+#include "hermes/SourceMap/SourceMapParser.h"
 #include "hermes/SourceMap/SourceMapTranslator.h"
 #include "hermes/Support/MemoryBuffer.h"
 #include "hermes/Support/SimpleDiagHandler.h"
@@ -87,7 +88,7 @@ std::pair<std::unique_ptr<BCProviderFromSrc>, std::string>
 BCProviderFromSrc::create(
     std::unique_ptr<Buffer> buffer,
     llvh::StringRef sourceURL,
-    std::unique_ptr<SourceMap> sourceMap,
+    llvh::StringRef sourceMap,
     const CompileFlags &compileFlags,
     llvh::StringRef topLevelFunctionName,
     SourceErrorManager::DiagHandlerTy diagHandler,
@@ -161,11 +162,26 @@ BCProviderFromSrc::create(
       buffer->size() >= context->getPreemptiveFileCompilationThreshold();
   int fileBufId = context->getSourceErrorManager().addNewSourceBuffer(
       std::make_unique<HermesLLVMMemoryBuffer>(std::move(buffer), sourceURL));
-  if (sourceMap != nullptr) {
+
+  if (!sourceMap.empty()) {
+    assert(sourceMap.back() == '\0' && "Must be null terminated");
+    // Convert the buffer into a form the parser needs. Note that the incoming
+    // buffer has the null terminator embedded at the end, whereas for the
+    // MemoryBuffer we want it past-the-end, so we shrink it by one.
+    llvh::MemoryBufferRef mbref(
+        llvh::StringRef{sourceMap.data(), sourceMap.size() - 1}, "");
+    SimpleDiagHandler diag;
+    SourceErrorManager sm;
+    diag.installInto(sm);
+    std::unique_ptr<SourceMap> parsedSM = SourceMapParser::parse(mbref, {}, sm);
+    if (!parsedSM) {
+      auto errorStr = diag.getErrorString();
+      return std::make_pair(nullptr, "Error parsing source map: " + errorStr);
+    }
     auto sourceMapTranslator =
         std::make_shared<SourceMapTranslator>(context->getSourceErrorManager());
     context->getSourceErrorManager().setTranslator(sourceMapTranslator);
-    sourceMapTranslator->addSourceMap(fileBufId, std::move(sourceMap));
+    sourceMapTranslator->addSourceMap(fileBufId, std::move(parsedSM));
   }
 
   auto parserMode = parser::FullParse;

lib/BCGen/HBC/HBC.cpp

@@ -31,7 +31,7 @@ void fullOptimizationPipeline(Module &M) {
 std::pair<std::unique_ptr<BCProvider>, std::string> createBCProviderFromSrc(
     std::unique_ptr<Buffer> buffer,
     llvh::StringRef sourceURL,
-    std::unique_ptr<SourceMap> sourceMap,
+    llvh::StringRef sourceMap,
     const CompileFlags &compileFlags,
     llvh::StringRef topLevelFunctionName,
     SourceErrorManager::DiagHandlerTy diagHandler,
@@ -41,7 +41,7 @@ std::pair<std::unique_ptr<BCProvider>, std::string> createBCProviderFromSrc(
   return BCProviderFromSrc::create(
       std::move(buffer),
       sourceURL,
-      std::move(sourceMap),
+      sourceMap,
       compileFlags,
       topLevelFunctionName,
       diagHandler,

lib/BCGen/HBC/HBCStub.cpp

@@ -23,7 +23,7 @@ void fullOptimizationPipeline(Module &M) {
 std::pair<std::unique_ptr<BCProvider>, std::string> createBCProviderFromSrc(
     std::unique_ptr<Buffer> buffer,
     llvh::StringRef sourceURL,
-    std::unique_ptr<SourceMap> sourceMap,
+    llvh::StringRef sourceMap,
     const CompileFlags &compileFlags,
     llvh::StringRef topLevelFunctionName,
     SourceErrorManager::DiagHandlerTy diagHandler,

lib/CMakeLists.txt

@@ -166,7 +166,6 @@ target_link_libraries(hermesvmlean_a PUBLIC
   hermesPublic_obj
   hermesRegex_obj
   hermesSupport_obj
-  hermesSourceMap_obj
   hermesVMRuntime_obj
   zip_obj
   )

lib/VM/JSLib/eval.cpp

@@ -96,7 +96,7 @@ CallResult<HermesValue> evalInEnvironment(
             bytecode_err = hbc::createBCProviderFromSrc(
                 std::move(buffer),
                 "",
-                nullptr,
+                {},
                 compileFlags,
                 "eval",
                 {},

lib/VM/Runtime.cpp

@@ -1019,7 +1019,7 @@ CallResult<HermesValue> Runtime::run(
     PerfSection loading("Loading new JavaScript code");
     loading.addArg("url", sourceURL);
     auto bytecode_err = hbc::createBCProviderFromSrc(
-        std::move(code), sourceURL, /* sourceMap */ nullptr, compileFlags);
+        std::move(code), sourceURL, /* sourceMap */ {}, compileFlags);
     if (!bytecode_err.first) {
       return raiseSyntaxError(TwineChar16(bytecode_err.second));
     }

tools/emhermesc/emhermesc.cpp

@@ -116,7 +116,7 @@ extern "C" CompileResult *hermesCompileToBytecode(
     const char *sourceMapData,
     size_t sourceMapSize) {
   auto compileRes = std::make_unique<CompileResult>();
-  std::unique_ptr<SourceMap> sourceMap;
+  llvh::StringRef smRef;
 
   if (source[sourceSize - 1] != 0) {
     compileRes->error_ = "Input source must be zero-terminated";
@@ -129,15 +129,7 @@ extern "C" CompileResult *hermesCompileToBytecode(
       return compileRes.release();
     }
 
-    SourceErrorManager sm;
-    SimpleDiagHandlerRAII diagHandler(sm);
-    sourceMap =
-        SourceMapParser::parse({sourceMapData, sourceMapSize - 1}, {}, sm);
-    if (!sourceMap) {
-      compileRes->error_ =
-          "Failed to parse source map:" + diagHandler.getErrorString();
-      return compileRes.release();
-    }
+    smRef = {sourceMapData, sourceMapSize};
   }
 
   hbc::CompileFlags flags{};
@@ -148,7 +140,7 @@ extern "C" CompileResult *hermesCompileToBytecode(
   auto res = hbc::createBCProviderFromSrc(
       std::make_unique<hermes::Buffer>((const uint8_t *)source, sourceSize - 1),
       sourceURL ? sourceURL : "",
-      std::move(sourceMap),
+      smRef,
       flags);
   if (!res.first) {
     if (!res.second.empty())