Broken minified code due to incorrect local variable renaming

[AshleyScirra]

Describe the bug

In some cases when lots of local variables are used, a local variable can be renamed to a name shared by a function parameter, even when that function parameter is also used in the local variable's scope. This means an identifier that used to refer to the function parameter now refers to the local variable instead.

To Reproduce

This is based on a sharpen filter where we discovered the problem. I've reduced it as much as possible while demonstrating the issue, but it is difficult to reduce this any further - it seems to depend on the number of variables used. For example if you remove r it does not reproduce any more.

function Sharpen(width, height, weights)
{
	for (let y = height - 1; y >= 0; --y)
	{
		for (let x = width - 1; x >= 0; --x)
		{
			let sy = y;
			let sx = x;
			let dstOff = (y * width + x) * 4;
			let r = 0;
			let g = 0;
			let b = 0;
			let a = 0;

			for (let cy = 0; cy < 3; cy++)
			{
				for (let cx = 0; cx < 3; cx++)
				{
					let scy = sy + cy - 1;
					let scx = sx + cx - 1;

					if (scy >= 0 && scy < height && scx >= 0 && scx < width)
					{
						let wt = weights[cy * 3 + cx];

						r += wt;
						g += wt;
						b += wt;
						a += wt;
					}
				}
			}

			console.log(r, g, b, dstOff);
		}
	}
}

Actual Output

function Sharpen(c, a, b) {
	for (let d = a - 1; 0 <= d; --d) for (let e = c - 1; 0 <= e; --e) {
		let f = d,
		    h = e,
		    i = 4 * (d * c + e),
		    j = 0,
		    k = 0,
		    g = 0,
		    l = 0;


		for (let a = 0; 3 > a; a++) for (let b = 0; 3 > b; b++) {
			let d = f + a - 1,
			    e = h + b - 1;


			if (0 <= d && d < a && 0 <= e && e < c) {
				let c = b[3 * a + b];	// <-- ERROR

				j += c, k += c, g += c, l += c;
			}
		}

		console.log(j, k, g, i);
	}
}

Expected Output The line: let c = b[3 * a + b]; was originally: let wt = weights[cy * 3 + cx]; Both weights (a function parameter) and cx (a local loop variable) have been renamed to b, even though they are both used in the same scope. The minified code has the same effect as if the original code had been written let wt = cx[cy * 3 + cx], which is obviously broken.

Configuration

Node.js 10.0.0 babel-core 6.26 babel-minify 0.4.3

This does not reproduce in the online REPL, but I have no idea which version that is using.

Possible solution

Disabling either babel-plugin-minify-mangle-names or babel-plugin-minify-simplify seems to avoid the problem, but neither is desirable for production minification. Alternatively you can make shotgun changes to the function until it compiles OK again, but it will inevitably crop up somewhere else again later down the line.

Additional context

In a large JavaScript application, this problem appears to be cropping up often enough to regularly break our production app in different and unexpected places, which is a serious problem for us.

[AshleyScirra]

Rolling back a few versions and found:

[email protected]: OK [email protected]: broken

So looks like a regression in 0.4.1.

[AshleyScirra]

Based on version range suspect this commit: https://github.com/babel/minify/commit/552766955c011618f86c0f39451b9e288383c67a

[Cyp]

I can't reproduce with latest master, but I'm not sure whether that means it's fixed, or just because the variables get named in a different order and it happens to not trigger.

Maybe due to the same bug as #867, which does trigger in master?

[fedetibaldo]

It is not fixed yet.

Describe the bug

Module pattern breaks immediately. Different variables with different names are output with the same letter.

To Reproduce

You can clone my repository and type npm i -D && npm run build. However, since links are not forever, here is some code as well

let loadingScreen = loadingScreen || (function() {

	"use strict";
	let expected = 0;
	let received = 0;

	let init = function(size, style = '') {
		if ( expected === 0 ) {
			expected = size;
			let container = document.querySelector('#ls-background');
			container.style = style;
			this.generateInnerHTML(container);
		}
	};
	let got = function(partial) {
		received += partial;
		const percentage = received / expected * 100;
		this.updateInnerHTML(percentage);
		if ( percentage === 100 ) {
			document.querySelector('#ls-main').classList.add('ls-active');
		}
	};
	
	let generateInnerHTML = function(container) {
		throw 'generateInnerHTML has not been implemented yet!';
	};
	let updateInnerHTML = function(percentage) {
		throw 'updateInnerHTML has not been implemented yet!';
	};

	return { init, got, generateInnerHTML, updateInnerHTML }

})();

Actual Output

var loadingScreen = loadingScreen || function () {

	"use strict";

	var a = 0;

	var b = 0;

	var d = function d(b) {
		var c = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : '';

		if (a === 0) {
			a = b;
			var e = document.querySelector('#ls-background');
			e.style = c;
			this.generateInnerHTML(e);
		}
	};

	var e = function e(c) {
		b += c;
		var d = b / a * 100;
		this.updateInnerHTML(d);
		if (d === 100) {
			document.querySelector('#ls-main').classList.add('ls-active');
		}
	};

	var b = function b(a) {
		throw 'generateInnerHTML has not been implemented yet!';
	};

	var b = function b(a) {
		throw 'updateInnerHTML has not been implemented yet!';
	};

	return { init: c, got: d, generateInnerHTML: e, updateInnerHTML: f };
}();

Expected Output

var loadingScreen = loadingScreen || function () {

	"use strict";

	var a = 0;

	var b = 0;

	var c = function c(b) {  // <-----------
		var c = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : '';

		if (a === 0) {
			a = b;
			var e = document.querySelector('#ls-background');
			e.style = c;
			this.generateInnerHTML(e);
		}
	};

	var d = function d(c) { // <-----------
		b += c;
		var d = b / a * 100;
		this.updateInnerHTML(d);
		if (d === 100) {
			document.querySelector('#ls-main').classList.add('ls-active');
		}
	};

	var e = function e(a) {  // <-----------
		throw 'generateInnerHTML has not been implemented yet!';
	};

	var f = function f(a) {  // <------------
		throw 'updateInnerHTML has not been implemented yet!';
	};

	return { init: c, got: d, generateInnerHTML: e, updateInnerHTML: f };
}();

Once again, variables within the same scope got duplicated and, additionally, variables in the same scope lose their consistency

Configuration

"babel-core": "^6.26.3",
"babel-plugin-minify-mangle-names": "^0.4.3",
"babel-preset-es2015-nostrict": "^6.6.2",
"gulp": "^4.0.0",
"gulp-babel": "^7.0.1",

This does not reproduce in the online REPL. On the contrary: it does a great job online, encapsulating the function declarations in the returned object.

[AshleyScirra]

This issue is about a year old and continues to be a problem for us.

Is there someone on the team I can pay to fix this?