Clearer diagnostics, no output after an error, and two UBSan fixes.

src/Error.cpp

@@ -109,8 +109,7 @@ std::string Error::toString() const
 		s << "openvcl: ";
 	}
 
-	if( m_warning )
-		s << "warning: ";
+	s << (m_warning ? "warning: " : "error: ");
 
 	s << m_string;
 

src/Parser.cpp

@@ -2101,6 +2101,12 @@ bool Parser::generateCode()
 
 bool Parser::writeOutput()
 {
+	// write nothing once an error has been reported: a .vsm left behind
+	// would look up to date to make, and the next build would use it
+
+	if( Error::HasErrors() )
+		return false;
+
 	if( m_cmdLine.output() == "-" )
 		return writeOutputStream( std::cout );
 
@@ -2171,7 +2177,9 @@ std::string Parser::tempFilename()
 	{
 		std::stringstream buffer;
 
-		buffer << "/tmp/vcl" << std::hex << std::setfill('0') << std::setw(8) << ((getpid()+rand()) << 8) + m_tempCounter++;
+		// unsigned, so the sum and the shift wrap instead of overflowing an int
+		buffer << "/tmp/vcl" << std::hex << std::setfill('0') << std::setw(8)
+		       << ((static_cast<unsigned int>(getpid()) + static_cast<unsigned int>(rand())) << 8) + m_tempCounter++;
 		std::string temp = buffer.str();
 
 		std::ofstream file(temp.c_str(),std::ifstream::out);

src/RegisterAllocator.cpp

@@ -269,9 +269,10 @@ bool RegisterAllocator::process( std::list<Token>& tokens )
 
 	if( m_aliases.size() > 0 )
 	{
-		if( !processAliases() )
+		Alias* unallocated = NULL;
+		if( !processAliases( unallocated ) )
 		{
-			Error::Display( Error( "Register allocation ran out of registers" ) );
+			reportUnallocatedAlias( tokens, unallocated );
 			return false;
 		}
 	}
@@ -559,7 +560,7 @@ bool RegisterAllocator::collectLabels( std::list<Token>::iterator start, std::li
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-bool RegisterAllocator::processAliases()
+bool RegisterAllocator::processAliases( Alias*& unallocated )
 {
 	// allocate registers that do not overlap usage
 
@@ -798,6 +799,7 @@ bool RegisterAllocator::processAliases()
 				std::cerr << "Failed to allocate " << (dest->type() == Alias::FLOAT ? "FLOAT" : "INTEGER")
 				          << " register for alias" << std::endl;
 			}
+			unallocated = dest;
 			return false;
 		}
 		//assert( dest->allocatedRegister() );
@@ -808,6 +810,49 @@ bool RegisterAllocator::processAliases()
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
+void RegisterAllocator::reportUnallocatedAlias( const std::list<Token>& tokens, const Alias* alias ) const
+{
+	if( !alias )
+	{
+		Error::Display( Error( "Register allocation ran out of registers" ) );
+		return;
+	}
+
+	// the registers .init_vf/.init_vi left to the allocator
+
+	const bool isFloat = (alias->type() == Alias::FLOAT);
+	unsigned int available = 0;
+	for( unsigned int i = 0; i < (isFloat ? 32u : 16u); ++i )
+	{
+		if( (isFloat ? m_floats[i] : m_integers[i]).available() )
+			++available;
+	}
+
+	std::stringstream message;
+	message << "Register allocation ran out of " << (isFloat ? "VF" : "VI")
+	        << " registers (" << available << " available)";
+
+	// name the variable, where it first appears
+
+	for( std::list<Token>::const_iterator i = tokens.begin(); i != tokens.end(); ++i )
+	{
+		for( std::list<Token::Argument>::const_iterator a = (*i).arguments().begin(); a != (*i).arguments().end(); ++a )
+		{
+			if( ((*a).content() == Token::Argument::ALIAS) && (*a).dependency()
+			    && ((*a).dependency()->alias() == alias) )
+			{
+				message << " for " << (*a).alias();
+				Error::Display( Error( message.str(), *i, *a ) );
+				return;
+			}
+		}
+	}
+
+	Error::Display( Error( message.str() ) );
+}
+
+///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
 namespace
 {
 	void extendContinuationBlockLiveRanges( std::list<Token>::iterator blockStart, std::list<Token>::iterator blockEnd )

src/RegisterAllocator.h

@@ -88,7 +88,8 @@ class RegisterAllocator
 
 	bool processCommonDirective( Token& token );
 
-	bool processAliases();
+	bool processAliases( Alias*& unallocated );
+	void reportUnallocatedAlias( const std::list<Token>& tokens, const Alias* alias ) const;
 
 	// Walks all emittable tokens and records, for each physical register,
 	// the line numbers where it appears as a *literal* (non-alias) operand.

src/Tokenizer.cpp

@@ -24,6 +24,7 @@ Tokenizer::Tokenizer()
 {
 	setVsmMode( false );
 	setComment( false );
+	setNewSyntax( false );
 
 	m_availableFloats = 0;
 	m_availableIntegers = 0;

test/CMakeLists.txt

@@ -69,6 +69,7 @@ set(OPENVCL_TEST_SOURCES
     ${CMAKE_CURRENT_SOURCE_DIR}/unit/test_vsm_cost_analyzer.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/integration/openvcl_runner.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_cli.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_diagnostics.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_init_vf_range.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_clip_validation.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/integration/test_clip_implicit_w.cpp

test/integration/test_diagnostics.cpp

@@ -0,0 +1,134 @@
+// Regression tests: openvcl's diagnostics are easy to find and act on.
+//
+// Errors were printed as "file(line) : message", without the "error:" tag
+// warnings already had, so searching a build log for "error:" found
+// nothing.  Running out of registers said only "Register allocation ran out
+// of registers", with no variable, register type or location.  And a failed
+// build still wrote its output file, which then looked up to date to make.
+
+#include "test_harness.h"
+#include "openvcl_runner.h"
+
+#include <cstdio>
+#include <cstdlib>
+#include <fstream>
+#include <sstream>
+#include <string>
+#include <unistd.h>
+#include <vector>
+
+namespace
+{
+    bool contains(const std::string& text, const std::string& needle)
+    {
+        return text.find(needle) != std::string::npos;
+    }
+
+    std::string program(const std::string& directives, const std::string& body)
+    {
+        return directives +
+            "\t.syntax new\n"
+            "\t.name vsmDiagnostics\n"
+            "\t--enter\n"
+            "\t--endenter\n"
+            + body +
+            "\t--exit\n"
+            "\t--endexit\n";
+    }
+
+    const char* kAllRegisters = "\t.init_vf_all\n\t.init_vi_all\n";
+
+    // Line 8 passes clipw a field other than W.
+    std::string invalidProgram()
+    {
+        return program(kAllRegisters,
+            "\tlq fA, 1(vi00)\n"
+            "\tclipw.xyz fA, fA[x]\n"
+            "\tfcand vi01, 0x3f\n"
+            "\tisw.x vi01, 2(vi00)\n");
+    }
+
+    std::string validProgram()
+    {
+        return program(kAllRegisters,
+            "\tlq fA, 1(vi00)\n"
+            "\tsq fA, 2(vi00)\n");
+    }
+}
+
+TEST_CASE("openvcl: an error at a source line is tagged error:")
+{
+    ::test::RunResult r = ::test::run_openvcl(std::vector<std::string>(), invalidProgram());
+
+    CHECK(r.exit_code != 0);
+    CHECK(contains(r.stderr_data, "(8) : error: Invalid argument (clipw.xyz fA, >>> fA[x] <<<"));
+}
+
+TEST_CASE("openvcl: an error without a location is tagged error:")
+{
+    std::vector<std::string> args;
+    args.push_back("-o");
+    args.push_back("/nonexistent-openvcl-dir/out.vsm");
+    ::test::RunResult r = ::test::run_openvcl(args, validProgram());
+
+    CHECK(r.exit_code != 0);
+    CHECK(contains(r.stderr_data, "openvcl: error: Could not open output file"));
+}
+
+TEST_CASE("openvcl: running out of VI registers names the variable and where it is set")
+{
+    // only two VI registers for three values live at once; line 9 sets the third
+    ::test::RunResult r = ::test::run_openvcl(std::vector<std::string>(), program(
+        "\t.init_vf_all\n\t.init_vi vi01, vi02\n",
+        "\tilw.x iA, 0(vi00)\n"
+        "\tilw.x iB, 1(vi00)\n"
+        "\tilw.x iC, 2(vi00)\n"
+        "\tiadd iD, iA, iB\n"
+        "\tiadd iE, iD, iC\n"
+        "\tisw.x iE, 3(vi00)\n"));
+
+    CHECK(r.exit_code != 0);
+    CHECK(contains(r.stderr_data, "(9) : error: Register allocation ran out of VI registers (2 available) for iC (ilw.x >>> iC <<<, 2(vi00))"));
+}
+
+TEST_CASE("openvcl: running out of VF registers names the variable and where it is set")
+{
+    ::test::RunResult r = ::test::run_openvcl(std::vector<std::string>(), program(
+        "\t.init_vf vf01-vf02\n\t.init_vi_all\n",
+        "\tlq fA, 0(vi00)\n"
+        "\tlq fB, 1(vi00)\n"
+        "\tlq fC, 2(vi00)\n"
+        "\tadd fD, fA, fB\n"
+        "\tadd fE, fD, fC\n"
+        "\tsq fE, 3(vi00)\n"));
+
+    CHECK(r.exit_code != 0);
+    CHECK(contains(r.stderr_data, "(9) : error: Register allocation ran out of VF registers (2 available) for fC (lq >>> fC <<<"));
+}
+
+TEST_CASE("openvcl: a failed build writes no output")
+{
+    char path[] = "/tmp/openvcl_diag_XXXXXX.vsm";
+    int fd = mkstemps(path, 4);
+    REQUIRE(fd >= 0);
+    const char stale[] = "stale output from an earlier build\n";
+    REQUIRE(write(fd, stale, sizeof(stale) - 1) == static_cast<ssize_t>(sizeof(stale) - 1));
+    close(fd);
+
+    std::vector<std::string> args;
+    args.push_back("-o");
+    args.push_back(path);
+    ::test::RunResult r = ::test::run_openvcl(args, invalidProgram());
+
+    std::ifstream f(path);
+    std::stringstream contents;
+    contents << f.rdbuf();
+    std::remove(path);
+
+    CHECK(r.exit_code != 0);
+    CHECK(contents.str() == stale);
+
+    ::test::RunResult toStdout = ::test::run_openvcl(std::vector<std::string>(), invalidProgram());
+    CHECK(toStdout.exit_code != 0);
+    CHECK(toStdout.stdout_data.empty());
+}