Skip to content

Commit

Permalink
fix #162 and #170: use entry point index as bit
Browse files Browse the repository at this point in the history 
This fixes a crash that happens when there are more than 8 entry points. The problem is due to some code that is meant to eventually be a part of the code splitting feature.

For code splitting, files are classified into separate chunks by which entry points they are reachable by. Reachability is determined using a bit set where the bit is identified as the index of the entry point in the entry points array.

Unfortunately I confused this index with the source index of the entry point, which is the index of the entry point in the overall array of sources, and which is much bigger. This causes a crash if there's an entry point with a source index of 8 or higher, since that tries to set a bit in the second byte of the bit set, which was never allocated because there's only ever one entry point in the current independent compilation mode (i.e. with code splitting disabled).
  • Loading branch information
@evanw
evanw committed Jun 4, 2020
1 parent a732214 commit f5e1804
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 5 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* Fix a crash with more than 8 entry points ([#162](https://github.com/evanw/esbuild/pull/162))

This bug was due to the wrong index being used for an internal bit set. That caused a crash due to an out-of-bounds array read when esbuild is run with more than 8 entry points. I now have test coverage for large numbers of entry points, so this should not happen again.

* Fix slash characters in file loader ([#164](https://github.com/evanw/esbuild/pull/164))

This fixes a bug where the base64-encoded hash included in the file name could sometimes contain a `/` character. The fix is to use the base64 character set for URL-encoding, which replaces the `/` character with a `_` character.
Expand Down
111 changes: 111 additions & 0 deletions internal/bundler/bundler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4612,3 +4612,114 @@ render(h(App, null), document.getElementById("app"));
},
})
}

// This test case makes sure many entry points don't cause a crash
func TestManyEntryPoints(t *testing.T) {
expectBundled(t, bundled{
files: map[string]string{
"/shared.js": `export default 123`,

"/e00.js": `import x from './shared'; console.log(x)`,
"/e01.js": `import x from './shared'; console.log(x)`,
"/e02.js": `import x from './shared'; console.log(x)`,
"/e03.js": `import x from './shared'; console.log(x)`,
"/e04.js": `import x from './shared'; console.log(x)`,
"/e05.js": `import x from './shared'; console.log(x)`,
"/e06.js": `import x from './shared'; console.log(x)`,
"/e07.js": `import x from './shared'; console.log(x)`,
"/e08.js": `import x from './shared'; console.log(x)`,
"/e09.js": `import x from './shared'; console.log(x)`,

"/e10.js": `import x from './shared'; console.log(x)`,
"/e11.js": `import x from './shared'; console.log(x)`,
"/e12.js": `import x from './shared'; console.log(x)`,
"/e13.js": `import x from './shared'; console.log(x)`,
"/e14.js": `import x from './shared'; console.log(x)`,
"/e15.js": `import x from './shared'; console.log(x)`,
"/e16.js": `import x from './shared'; console.log(x)`,
"/e17.js": `import x from './shared'; console.log(x)`,
"/e18.js": `import x from './shared'; console.log(x)`,
"/e19.js": `import x from './shared'; console.log(x)`,

"/e20.js": `import x from './shared'; console.log(x)`,
"/e21.js": `import x from './shared'; console.log(x)`,
"/e22.js": `import x from './shared'; console.log(x)`,
"/e23.js": `import x from './shared'; console.log(x)`,
"/e24.js": `import x from './shared'; console.log(x)`,
"/e25.js": `import x from './shared'; console.log(x)`,
"/e26.js": `import x from './shared'; console.log(x)`,
"/e27.js": `import x from './shared'; console.log(x)`,
"/e28.js": `import x from './shared'; console.log(x)`,
"/e29.js": `import x from './shared'; console.log(x)`,

"/e30.js": `import x from './shared'; console.log(x)`,
"/e31.js": `import x from './shared'; console.log(x)`,
"/e32.js": `import x from './shared'; console.log(x)`,
"/e33.js": `import x from './shared'; console.log(x)`,
"/e34.js": `import x from './shared'; console.log(x)`,
"/e35.js": `import x from './shared'; console.log(x)`,
"/e36.js": `import x from './shared'; console.log(x)`,
"/e37.js": `import x from './shared'; console.log(x)`,
"/e38.js": `import x from './shared'; console.log(x)`,
"/e39.js": `import x from './shared'; console.log(x)`,
},
entryPaths: []string{
"/e00.js", "/e01.js", "/e02.js", "/e03.js", "/e04.js", "/e05.js", "/e06.js", "/e07.js", "/e08.js", "/e09.js",
"/e10.js", "/e11.js", "/e12.js", "/e13.js", "/e14.js", "/e15.js", "/e16.js", "/e17.js", "/e18.js", "/e19.js",
"/e20.js", "/e21.js", "/e22.js", "/e23.js", "/e24.js", "/e25.js", "/e26.js", "/e27.js", "/e28.js", "/e29.js",
"/e30.js", "/e31.js", "/e32.js", "/e33.js", "/e34.js", "/e35.js", "/e36.js", "/e37.js", "/e38.js", "/e39.js",
},
parseOptions: parser.ParseOptions{
IsBundling: true,
},
bundleOptions: BundleOptions{
IsBundling: true,
AbsOutputDir: "/out",
},
expected: map[string]string{
"/out/e00.js": "// /shared.js\nconst shared_default = 123;\n\n// /e00.js\nconsole.log(shared_default);\n",
"/out/e01.js": "// /shared.js\nconst shared_default = 123;\n\n// /e01.js\nconsole.log(shared_default);\n",
"/out/e02.js": "// /shared.js\nconst shared_default = 123;\n\n// /e02.js\nconsole.log(shared_default);\n",
"/out/e03.js": "// /shared.js\nconst shared_default = 123;\n\n// /e03.js\nconsole.log(shared_default);\n",
"/out/e04.js": "// /shared.js\nconst shared_default = 123;\n\n// /e04.js\nconsole.log(shared_default);\n",
"/out/e05.js": "// /shared.js\nconst shared_default = 123;\n\n// /e05.js\nconsole.log(shared_default);\n",
"/out/e06.js": "// /shared.js\nconst shared_default = 123;\n\n// /e06.js\nconsole.log(shared_default);\n",
"/out/e07.js": "// /shared.js\nconst shared_default = 123;\n\n// /e07.js\nconsole.log(shared_default);\n",
"/out/e08.js": "// /shared.js\nconst shared_default = 123;\n\n// /e08.js\nconsole.log(shared_default);\n",
"/out/e09.js": "// /shared.js\nconst shared_default = 123;\n\n// /e09.js\nconsole.log(shared_default);\n",

"/out/e10.js": "// /shared.js\nconst shared_default = 123;\n\n// /e10.js\nconsole.log(shared_default);\n",
"/out/e11.js": "// /shared.js\nconst shared_default = 123;\n\n// /e11.js\nconsole.log(shared_default);\n",
"/out/e12.js": "// /shared.js\nconst shared_default = 123;\n\n// /e12.js\nconsole.log(shared_default);\n",
"/out/e13.js": "// /shared.js\nconst shared_default = 123;\n\n// /e13.js\nconsole.log(shared_default);\n",
"/out/e14.js": "// /shared.js\nconst shared_default = 123;\n\n// /e14.js\nconsole.log(shared_default);\n",
"/out/e15.js": "// /shared.js\nconst shared_default = 123;\n\n// /e15.js\nconsole.log(shared_default);\n",
"/out/e16.js": "// /shared.js\nconst shared_default = 123;\n\n// /e16.js\nconsole.log(shared_default);\n",
"/out/e17.js": "// /shared.js\nconst shared_default = 123;\n\n// /e17.js\nconsole.log(shared_default);\n",
"/out/e18.js": "// /shared.js\nconst shared_default = 123;\n\n// /e18.js\nconsole.log(shared_default);\n",
"/out/e19.js": "// /shared.js\nconst shared_default = 123;\n\n// /e19.js\nconsole.log(shared_default);\n",

"/out/e20.js": "// /shared.js\nconst shared_default = 123;\n\n// /e20.js\nconsole.log(shared_default);\n",
"/out/e21.js": "// /shared.js\nconst shared_default = 123;\n\n// /e21.js\nconsole.log(shared_default);\n",
"/out/e22.js": "// /shared.js\nconst shared_default = 123;\n\n// /e22.js\nconsole.log(shared_default);\n",
"/out/e23.js": "// /shared.js\nconst shared_default = 123;\n\n// /e23.js\nconsole.log(shared_default);\n",
"/out/e24.js": "// /shared.js\nconst shared_default = 123;\n\n// /e24.js\nconsole.log(shared_default);\n",
"/out/e25.js": "// /shared.js\nconst shared_default = 123;\n\n// /e25.js\nconsole.log(shared_default);\n",
"/out/e26.js": "// /shared.js\nconst shared_default = 123;\n\n// /e26.js\nconsole.log(shared_default);\n",
"/out/e27.js": "// /shared.js\nconst shared_default = 123;\n\n// /e27.js\nconsole.log(shared_default);\n",
"/out/e28.js": "// /shared.js\nconst shared_default = 123;\n\n// /e28.js\nconsole.log(shared_default);\n",
"/out/e29.js": "// /shared.js\nconst shared_default = 123;\n\n// /e29.js\nconsole.log(shared_default);\n",

"/out/e30.js": "// /shared.js\nconst shared_default = 123;\n\n// /e30.js\nconsole.log(shared_default);\n",
"/out/e31.js": "// /shared.js\nconst shared_default = 123;\n\n// /e31.js\nconsole.log(shared_default);\n",
"/out/e32.js": "// /shared.js\nconst shared_default = 123;\n\n// /e32.js\nconsole.log(shared_default);\n",
"/out/e33.js": "// /shared.js\nconst shared_default = 123;\n\n// /e33.js\nconsole.log(shared_default);\n",
"/out/e34.js": "// /shared.js\nconst shared_default = 123;\n\n// /e34.js\nconsole.log(shared_default);\n",
"/out/e35.js": "// /shared.js\nconst shared_default = 123;\n\n// /e35.js\nconsole.log(shared_default);\n",
"/out/e36.js": "// /shared.js\nconst shared_default = 123;\n\n// /e36.js\nconsole.log(shared_default);\n",
"/out/e37.js": "// /shared.js\nconst shared_default = 123;\n\n// /e37.js\nconsole.log(shared_default);\n",
"/out/e38.js": "// /shared.js\nconst shared_default = 123;\n\n// /e38.js\nconsole.log(shared_default);\n",
"/out/e39.js": "// /shared.js\nconst shared_default = 123;\n\n// /e39.js\nconsole.log(shared_default);\n",
},
})
}
10 changes: 5 additions & 5 deletions internal/bundler/linker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -981,8 +981,8 @@ func (c *linkerContext) markPartsReachableFromEntryPoints() {
}

// Each entry point marks all files reachable from itself
for _, entryPoint := range c.entryPoints {
c.includeFile(entryPoint, uint(entryPoint), 0)
for i, entryPoint := range c.entryPoints {
c.includeFile(entryPoint, uint(i), 0)
}
}

Expand Down Expand Up @@ -1145,7 +1145,7 @@ func (c *linkerContext) computeChunks() []chunkMeta {
// Create a chunk for the entry point here to ensure that the chunk is
// always generated even if the resulting file is empty
entryBits := newBitSet(uint(len(c.entryPoints)))
entryBits.setBit(uint(entryPoint))
entryBits.setBit(uint(i))
chunks[string(entryBits.entries)] = chunkMeta{
entryBits: entryBits,
hashbang: c.files[entryPoint].ast.Hashbang,
Expand All @@ -1165,8 +1165,8 @@ func (c *linkerContext) computeChunks() []chunkMeta {
chunk, ok := chunks[key]
if !ok {
// Initialize the chunk for the first time
for i, entryPoint := range c.entryPoints {
if partMeta.entryBits.hasBit(uint(entryPoint)) {
for i, _ := range c.entryPoints {
if partMeta.entryBits.hasBit(uint(i)) {
if chunk.name != "" {
chunk.name = c.stripKnownFileExtension(chunk.name) + "_"
}
Expand Down

0 comments on commit f5e1804

Please sign in to comment.