This is an archive of the discontinued LLVM Phabricator instance.

[Analyzer] Check function name size before performing indexing
ClosedPublic

Authored by george.karpenkov on Sep 15 2017, 10:53 AM.

Reviewers

This revision is now accepted and ready to land.Sep 15 2017, 10:57 AM

I think it would good to add a test for the changed functionality.

Looks like I rushed into accepting. This needs a test case of cause!

@dcoughlin that would be OK?

Looks good to me. Thanks for fixing this!

Committed in r313385

Neat! I've seen this code multiple times but never noticed this bug.

test/Analysis/retain-release.mm
465	There's also the "// no-crash" thingy we write from time to time in order to quickly explain what the test is about.

Path

Size

lib/

StaticAnalyzer/

Checkers/

3 lines

test/

Analysis/

2 lines

	Show First 20 Lines • Show All 991 Lines • ▼ Show 20 Lines
	break;			break;
	}			}

	break;			break;
	}			}

	// Check for release functions, the only kind of functions that we care			// Check for release functions, the only kind of functions that we care
	// about that don't return a pointer type.			// about that don't return a pointer type.
	if (FName[0] == 'C' && (FName[1] == 'F' \|\| FName[1] == 'G')) {			if (FName.size() >= 2 &&
				FName[0] == 'C' && (FName[1] == 'F' \|\| FName[1] == 'G')) {
	// Test for 'CGCF'.			// Test for 'CGCF'.
	FName = FName.substr(FName.startswith("CGCF") ? 4 : 2);			FName = FName.substr(FName.startswith("CGCF") ? 4 : 2);

	if (isRelease(FD, FName))			if (isRelease(FD, FName))
	S = getUnarySummary(FT, cfrelease);			S = getUnarySummary(FT, cfrelease);
	else {			else {
	assert (ScratchArgs.isEmpty());			assert (ScratchArgs.isEmpty());
	// Remaining CoreFoundation and CoreGraphics functions.			// Remaining CoreFoundation and CoreGraphics functions.
	▲ Show 20 Lines • Show All 991 Lines • Show Last 20 Lines

	Show First 20 Lines • Show All 455 Lines • ▼ Show 20 Lines

	void radar13722286::PrepareBitmap() {			void radar13722286::PrepareBitmap() {
	if (mStr.get() != 0) {			if (mStr.get() != 0) {
	Holder<const struct __CFString> str1;			Holder<const struct __CFString> str1;
	mStr.reset( CFStringCreateCopy( 0, str1.get() ) ); //expected-warning {{Potential leak of an object}}			mStr.reset( CFStringCreateCopy( 0, str1.get() ) ); //expected-warning {{Potential leak of an object}}
	}			}
	}			}

				// rdar://34210609
				void _() { _(); }; // no-warning
				NoQUnsubmitted Not Done Reply Inline Actions There's also the "// no-crash" thingy we write from time to time in order to quickly explain what the test is about. NoQ: There's also the "// no-crash" thingy we write from time to time in order to quickly explain…