Revert "Fix unit test for setting locale creating malformed HLSL shader code"
This reverts commit 5f662c0042703344eb0eef6d1c123e902e3aefbf.
Reason for revert: Some recent crashes related to streams, could be
related to this change.
https://bugs.chromium.org/p/chromium/issues/detail?id=932359
Original change's description:
> Fix unit test for setting locale creating malformed HLSL shader code
> Fix malformed HLSL shader code in other locales than classic
>
> Bug: angleproject:1433
> Change-Id: I30bad0bd0cfda465ec7200e48e12800d7d8efd26
> Reviewed-on: https://chromium-review.googlesource.com/c/1447862
> Reviewed-by: Geoff Lang <geofflang@chromium.org>
> Reviewed-by: Jamie Madill <jmadill@chromium.org>
> Commit-Queue: Jonah Ryan-Davis <jonahr@google.com>
TBR=geofflang@chromium.org,jmadill@chromium.org,jonahr@google.com
Bug: angleproject:1433
Change-Id: I28caa073b3996a95f3f233bed9dc91dc44263835
Reviewed-on: https://chromium-review.googlesource.com/c/1474624
Reviewed-by: Jonah Ryan-Davis <jonahr@google.com>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Geoff Lang <geofflang@chromium.org>
diff --git a/src/compiler/translator/AtomicCounterFunctionHLSL.cpp b/src/compiler/translator/AtomicCounterFunctionHLSL.cpp
index 9f8e4fd..63fe026 100644
--- a/src/compiler/translator/AtomicCounterFunctionHLSL.cpp
+++ b/src/compiler/translator/AtomicCounterFunctionHLSL.cpp
@@ -9,7 +9,6 @@
#include "compiler/translator/AtomicCounterFunctionHLSL.h"
-#include "compiler/translator/Common.h"
#include "compiler/translator/ImmutableStringBuilder.h"
#include "compiler/translator/InfoSink.h"
#include "compiler/translator/IntermNode.h"
@@ -93,7 +92,7 @@
ImmutableString getAtomicCounterNameForBinding(int binding)
{
- std::stringstream counterName = sh::InitializeStream<std::stringstream>();
+ std::stringstream counterName;
counterName << kAtomicCounterBaseName << binding;
return ImmutableString(counterName.str());
}
diff --git a/src/compiler/translator/BuiltInFunctionEmulatorGLSL.cpp b/src/compiler/translator/BuiltInFunctionEmulatorGLSL.cpp
index dc48527..7c619dd 100644
--- a/src/compiler/translator/BuiltInFunctionEmulatorGLSL.cpp
+++ b/src/compiler/translator/BuiltInFunctionEmulatorGLSL.cpp
@@ -5,7 +5,6 @@
//
#include "compiler/translator/BuiltInFunctionEmulatorGLSL.h"
-
#include "angle_gl.h"
#include "compiler/translator/BuiltInFunctionEmulator.h"
#include "compiler/translator/VersionGLSL.h"
@@ -88,7 +87,7 @@
};
for (int dim = 2; dim <= 4; ++dim)
{
- std::stringstream ss = sh::InitializeStream<std::stringstream>();
+ std::stringstream ss;
ss << "emu_precision vec" << dim << " atan_emu(emu_precision vec" << dim
<< " y, emu_precision vec" << dim << " x)\n"
<< "{\n"
diff --git a/src/compiler/translator/CallDAG.cpp b/src/compiler/translator/CallDAG.cpp
index 63c8cd0..9c0e2cf 100644
--- a/src/compiler/translator/CallDAG.cpp
+++ b/src/compiler/translator/CallDAG.cpp
@@ -171,7 +171,7 @@
InitResult result = INITDAG_SUCCESS;
- std::stringstream errorStream = sh::InitializeStream<std::stringstream>();
+ std::stringstream errorStream;
while (!functionsToProcess.empty())
{
diff --git a/src/compiler/translator/Common.h b/src/compiler/translator/Common.h
index 179cfe2..d0b8347 100644
--- a/src/compiler/translator/Common.h
+++ b/src/compiler/translator/Common.h
@@ -125,15 +125,6 @@
return buffer;
}
-// Initialize a new stream which must be imbued with the classic locale
-template <typename T>
-T InitializeStream()
-{
- T stream;
- stream.imbue(std::locale::classic());
- return stream;
-}
-
} // namespace sh
namespace std
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index 685a1ae..771d050 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -68,7 +68,7 @@
{
static int fileIndex = 0;
- std::ostringstream o = sh::InitializeStream<std::ostringstream>();
+ std::ostringstream o;
o << "corpus/" << fileIndex++ << ".sample";
std::string s = o.str();
@@ -940,7 +940,7 @@
void TCompiler::setResourceString()
{
- std::ostringstream strstream = sh::InitializeStream<std::ostringstream>();
+ std::ostringstream strstream;
// clang-format off
strstream << ":MaxVertexAttribs:" << mResources.MaxVertexAttribs
@@ -1107,7 +1107,7 @@
if (depth >= mResources.MaxCallStackDepth)
{
// Trace back the function chain to have a meaningful info log.
- std::stringstream errorStream = sh::InitializeStream<std::stringstream>();
+ std::stringstream errorStream;
errorStream << "Call stack too deep (larger than " << mResources.MaxCallStackDepth
<< ") with the following call chain: "
<< record.node->getFunction()->name();
diff --git a/src/compiler/translator/DirectiveHandler.cpp b/src/compiler/translator/DirectiveHandler.cpp
index e468fd4..46e67f5 100644
--- a/src/compiler/translator/DirectiveHandler.cpp
+++ b/src/compiler/translator/DirectiveHandler.cpp
@@ -10,7 +10,6 @@
#include "angle_gl.h"
#include "common/debug.h"
-#include "compiler/translator/Common.h"
#include "compiler/translator/Diagnostics.h"
namespace sh
@@ -191,7 +190,7 @@
}
else
{
- std::stringstream stream = sh::InitializeStream<std::stringstream>();
+ std::stringstream stream;
stream << version;
std::string str = stream.str();
mDiagnostics.error(loc, "version number not supported", str.c_str());
diff --git a/src/compiler/translator/InfoSink.cpp b/src/compiler/translator/InfoSink.cpp
index e8a8453..fc19d7e 100644
--- a/src/compiler/translator/InfoSink.cpp
+++ b/src/compiler/translator/InfoSink.cpp
@@ -69,7 +69,7 @@
void TInfoSinkBase::location(int file, int line)
{
- TPersistStringStream stream = sh::InitializeStream<TPersistStringStream>();
+ TPersistStringStream stream;
if (line)
stream << file << ":" << line;
else
diff --git a/src/compiler/translator/InfoSink.h b/src/compiler/translator/InfoSink.h
index 100d06e..44f0e83 100644
--- a/src/compiler/translator/InfoSink.h
+++ b/src/compiler/translator/InfoSink.h
@@ -41,7 +41,7 @@
template <typename T>
TInfoSinkBase &operator<<(const T &t)
{
- TPersistStringStream stream = sh::InitializeStream<TPersistStringStream>();
+ TPersistStringStream stream;
stream << t;
sink.append(stream.str());
return *this;
@@ -79,7 +79,7 @@
// does not have a fractional part, the default precision format does
// not write the decimal portion which gets interpreted as integer by
// the compiler.
- TPersistStringStream stream = sh::InitializeStream<TPersistStringStream>();
+ TPersistStringStream stream;
if (fractionalPart(f) == 0.0f)
{
stream.precision(1);
diff --git a/src/compiler/translator/OutputHLSL.cpp b/src/compiler/translator/OutputHLSL.cpp
index e0412cb..84e32dd 100644
--- a/src/compiler/translator/OutputHLSL.cpp
+++ b/src/compiler/translator/OutputHLSL.cpp
@@ -39,7 +39,7 @@
TString ArrayHelperFunctionName(const char *prefix, const TType &type)
{
- TStringStream fnName = sh::InitializeStream<TStringStream>();
+ TStringStream fnName;
fnName << prefix << "_";
if (type.isArray())
{
@@ -132,7 +132,7 @@
constexpr int kZeroCount = 256;
std::string DefineZeroArray()
{
- std::stringstream ss = sh::InitializeStream<std::stringstream>();
+ std::stringstream ss;
// For 'static', if the declaration does not include an initializer, the value is set to zero.
// https://docs.microsoft.com/en-us/windows/desktop/direct3dhlsl/dx-graphics-hlsl-variable-syntax
ss << "static uint " << kZeros << "[" << kZeroCount << "];\n";
@@ -141,7 +141,7 @@
std::string GetZeroInitializer(size_t size)
{
- std::stringstream ss = sh::InitializeStream<std::stringstream>();
+ std::stringstream ss;
size_t quotient = size / kZeroCount;
size_t reminder = size % kZeroCount;
@@ -416,7 +416,7 @@
init += indentString + "{\n";
for (unsigned int arrayIndex = 0u; arrayIndex < type.getOutermostArraySize(); ++arrayIndex)
{
- TStringStream indexedString = sh::InitializeStream<TStringStream>();
+ TStringStream indexedString;
indexedString << name << "[" << arrayIndex << "]";
TType elementType = type;
elementType.toArrayElementType();
@@ -891,8 +891,8 @@
out << kImage2DFunctionString << "\n";
- std::ostringstream systemValueDeclaration = sh::InitializeStream<std::ostringstream>();
- std::ostringstream glBuiltinInitialization = sh::InitializeStream<std::ostringstream>();
+ std::ostringstream systemValueDeclaration;
+ std::ostringstream glBuiltinInitialization;
systemValueDeclaration << "\nstruct CS_INPUT\n{\n";
glBuiltinInitialization << "\nvoid initGLBuiltins(CS_INPUT input)\n"
@@ -1900,7 +1900,7 @@
{
int index = nodeBinary->getRight()->getAsConstantUnion()->getIConst(0);
- std::stringstream prefixSink = sh::InitializeStream<std::stringstream>();
+ std::stringstream prefixSink;
prefixSink << samplerNamePrefixFromStruct(nodeBinary->getLeft()) << "_" << index;
return ImmutableString(prefixSink.str());
}
@@ -1910,7 +1910,7 @@
int index = nodeBinary->getRight()->getAsConstantUnion()->getIConst(0);
const TField *field = s->fields()[index];
- std::stringstream prefixSink = sh::InitializeStream<std::stringstream>();
+ std::stringstream prefixSink;
prefixSink << samplerNamePrefixFromStruct(nodeBinary->getLeft()) << "_"
<< field->name();
return ImmutableString(prefixSink.str());
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 4b92375..fcaca76 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -611,7 +611,7 @@
return true;
}
- std::stringstream reasonStream = sh::InitializeStream<std::stringstream>();
+ std::stringstream reasonStream;
reasonStream << "l-value required";
if (!message.empty())
{
@@ -902,7 +902,7 @@
{
if (ContainsSampler(pType.userDef))
{
- std::stringstream reasonStream = sh::InitializeStream<std::stringstream>();
+ std::stringstream reasonStream;
reasonStream << reason << " (structure contains a sampler)";
std::string reasonStr = reasonStream.str();
error(line, reasonStr.c_str(), getBasicString(pType.type));
@@ -3066,7 +3066,7 @@
if (mComputeShaderLocalSize[i] < 1 ||
mComputeShaderLocalSize[i] > maxComputeWorkGroupSizeValue)
{
- std::stringstream reasonStream = sh::InitializeStream<std::stringstream>();
+ std::stringstream reasonStream;
reasonStream << "invalid value: Value must be at least 1 and no greater than "
<< maxComputeWorkGroupSizeValue;
const std::string &reason = reasonStream.str();
@@ -3891,7 +3891,7 @@
// one to the field's struct nesting.
if (1 + field.type()->getDeepestStructNesting() > kWebGLMaxStructNesting)
{
- std::stringstream reasonStream = sh::InitializeStream<std::stringstream>();
+ std::stringstream reasonStream;
if (field.type()->getStruct()->symbolType() == SymbolType::Empty)
{
// This may happen in case there are nested struct definitions. While they are also
@@ -4085,7 +4085,7 @@
ASSERT(index >= 0);
if (index >= arraySize)
{
- std::stringstream reasonStream = sh::InitializeStream<std::stringstream>();
+ std::stringstream reasonStream;
reasonStream << reason << " '" << index << "'";
std::string token = reasonStream.str();
outOfRangeError(outOfRangeIndexIsError, location, reason, "[]");
@@ -4387,7 +4387,7 @@
checkLayoutQualifierSupported(qualifierTypeLine, qualifierType, 310);
if (intValue < 1)
{
- std::stringstream reasonStream = sh::InitializeStream<std::stringstream>();
+ std::stringstream reasonStream;
reasonStream << "out of range: " << getWorkGroupSizeString(index) << " must be positive";
std::string reason = reasonStream.str();
error(intValueLine, reason.c_str(), intValueString.c_str());
@@ -5672,7 +5672,7 @@
int offsetValue = values[i].getIConst();
if (offsetValue > maxOffsetValue || offsetValue < minOffsetValue)
{
- std::stringstream tokenStream = sh::InitializeStream<std::stringstream>();
+ std::stringstream tokenStream;
tokenStream << offsetValue;
std::string token = tokenStream.str();
error(offset->getLine(), "Texture offset value out of valid range",
diff --git a/src/compiler/translator/Symbol.cpp b/src/compiler/translator/Symbol.cpp
index 52ad4f3..babda8c 100644
--- a/src/compiler/translator/Symbol.cpp
+++ b/src/compiler/translator/Symbol.cpp
@@ -115,7 +115,7 @@
const TType *fieldType = field->type();
if (IsSampler(fieldType->getBasicType()) || fieldType->isStructureContainingSamplers())
{
- std::stringstream fieldName = sh::InitializeStream<std::stringstream>();
+ std::stringstream fieldName;
fieldName << namePrefix << "_" << field->name();
TString fieldApiName = apiNamePrefix + ".";
fieldApiName += field->name().data();
diff --git a/src/compiler/translator/Types.cpp b/src/compiler/translator/Types.cpp
index 9d0374b..6fc5875 100644
--- a/src/compiler/translator/Types.cpp
+++ b/src/compiler/translator/Types.cpp
@@ -756,7 +756,7 @@
elementType.toArrayElementType();
for (unsigned int arrayIndex = 0u; arrayIndex < getOutermostArraySize(); ++arrayIndex)
{
- std::stringstream elementName = sh::InitializeStream<std::stringstream>();
+ std::stringstream elementName;
elementName << namePrefix << "_" << arrayIndex;
TStringStream elementApiName;
elementApiName << apiNamePrefix << "[" << arrayIndex << "]";
diff --git a/src/compiler/translator/ValidateOutputs.cpp b/src/compiler/translator/ValidateOutputs.cpp
index 9880255..8115523 100644
--- a/src/compiler/translator/ValidateOutputs.cpp
+++ b/src/compiler/translator/ValidateOutputs.cpp
@@ -119,7 +119,7 @@
const size_t offsetLocation = location + elementIndex;
if ((*validOutputsToUse)[offsetLocation])
{
- std::stringstream strstr = sh::InitializeStream<std::stringstream>();
+ std::stringstream strstr;
strstr << "conflicting output locations with previously defined output '"
<< (*validOutputsToUse)[offsetLocation]->getName() << "'";
error(*symbol, strstr.str().c_str(), diagnostics);
diff --git a/src/compiler/translator/ValidateVaryingLocations.cpp b/src/compiler/translator/ValidateVaryingLocations.cpp
index 394e35c..55f6dbe 100644
--- a/src/compiler/translator/ValidateVaryingLocations.cpp
+++ b/src/compiler/translator/ValidateVaryingLocations.cpp
@@ -83,7 +83,7 @@
const int offsetLocation = location + elementIndex;
if (locationMap.find(offsetLocation) != locationMap.end())
{
- std::stringstream strstr = sh::InitializeStream<std::stringstream>();
+ std::stringstream strstr;
strstr << "'" << varying->getName()
<< "' conflicting location with previously defined '"
<< locationMap[offsetLocation]->getName() << "'";
diff --git a/src/compiler/translator/blocklayout.cpp b/src/compiler/translator/blocklayout.cpp
index 7d0609e..58c4f7b 100644
--- a/src/compiler/translator/blocklayout.cpp
+++ b/src/compiler/translator/blocklayout.cpp
@@ -11,7 +11,6 @@
#include "common/mathutil.h"
#include "common/utilities.h"
-#include "compiler/translator/Common.h"
namespace sh
{
@@ -133,7 +132,7 @@
std::string CollapseNameStack(const std::vector<std::string> &nameStack)
{
- std::stringstream strstr = sh::InitializeStream<std::stringstream>();
+ std::stringstream strstr;
for (const std::string &part : nameStack)
{
strstr << part;
@@ -416,7 +415,7 @@
void VariableNameVisitor::enterArrayElement(const ShaderVariable &arrayVar,
unsigned int arrayElement)
{
- std::stringstream strstr = sh::InitializeStream<std::stringstream>();
+ std::stringstream strstr;
strstr << "[" << arrayElement << "]";
std::string elementString = strstr.str();
mNameStack.push_back(elementString);
diff --git a/src/compiler/translator/tree_ops/EmulatePrecision.cpp b/src/compiler/translator/tree_ops/EmulatePrecision.cpp
index ba19bd1..ca4cf4a 100644
--- a/src/compiler/translator/tree_ops/EmulatePrecision.cpp
+++ b/src/compiler/translator/tree_ops/EmulatePrecision.cpp
@@ -174,7 +174,7 @@
std::string RoundingHelperWriterESSL::getTypeString(const char *glslType)
{
- std::stringstream typeStrStr = sh::InitializeStream<std::stringstream>();
+ std::stringstream typeStrStr;
typeStrStr << "highp " << glslType;
return typeStrStr.str();
}
@@ -257,7 +257,7 @@
void RoundingHelperWriterGLSL::writeVectorRoundingHelpers(TInfoSinkBase &sink,
const unsigned int size)
{
- std::stringstream vecTypeStrStr = sh::InitializeStream<std::stringstream>();
+ std::stringstream vecTypeStrStr;
vecTypeStrStr << "vec" << size;
std::string vecType = getTypeString(vecTypeStrStr.str().c_str());
@@ -287,7 +287,7 @@
const unsigned int rows,
const char *functionName)
{
- std::stringstream matTypeStrStr = sh::InitializeStream<std::stringstream>();
+ std::stringstream matTypeStrStr;
matTypeStrStr << "mat" << columns;
if (rows != columns)
{
@@ -379,7 +379,7 @@
void RoundingHelperWriterHLSL::writeVectorRoundingHelpers(TInfoSinkBase &sink,
const unsigned int size)
{
- std::stringstream vecTypeStrStr = sh::InitializeStream<std::stringstream>();
+ std::stringstream vecTypeStrStr;
vecTypeStrStr << "float" << size;
std::string vecType = vecTypeStrStr.str();
@@ -409,7 +409,7 @@
const unsigned int rows,
const char *functionName)
{
- std::stringstream matTypeStrStr = sh::InitializeStream<std::stringstream>();
+ std::stringstream matTypeStrStr;
matTypeStrStr << "float" << columns << "x" << rows;
std::string matType = matTypeStrStr.str();
@@ -744,7 +744,7 @@
TIntermTyped *right,
const char *opNameStr)
{
- std::stringstream strstr = sh::InitializeStream<std::stringstream>();
+ std::stringstream strstr;
if (left->getPrecision() == EbpMedium)
strstr << "angle_compound_" << opNameStr << "_frm";
else
diff --git a/src/tests/compiler_tests/ShCompile_test.cpp b/src/tests/compiler_tests/ShCompile_test.cpp
index 49ea0f8..d33b751 100644
--- a/src/tests/compiler_tests/ShCompile_test.cpp
+++ b/src/tests/compiler_tests/ShCompile_test.cpp
@@ -117,59 +117,39 @@
}
// Parsing floats in shaders can run afoul of locale settings.
-// Eg. in de_DE, `strtof("1.9")` will yield `1.0f`. (It's expecting "1,9")
+// In de_DE, `strtof("1.9")` will yield `1.0f`. (It's expecting "1,9")
TEST_F(ShCompileTest, DecimalSepLocale)
{
- // Locale names are platform dependent, add platform-specific names of locales to be tested here
- const std::string availableLocales[] = {
- "de_DE", "de-DE", "de_DE.UTF-8", "de_DE.ISO8859-1", "de_DE.ISO8859-15", "de_DE@euro",
- "de_DE.88591", "de_DE.88591.en", "de_DE.iso88591", "de_DE.ISO-8859-1", "de_DE.ISO_8859-1",
- "de_DE.iso885915", "de_DE.ISO-8859-15", "de_DE.ISO_8859-15", "de_DE.8859-15",
- "de_DE.8859-15@euro", "de_DE.ISO-8859-15@euro", "de_DE.UTF-8@euro", "de_DE.utf8",
- "German_germany", "German_Germany", "German_Germany.1252", "German_Germany.UTF-8", "German",
- // One ubuntu tester doesn't have a german locale, but da_DK.utf8 has similar float
- // representation
- "da_DK.utf8"};
+ const auto defaultLocale = setlocale(LC_NUMERIC, nullptr);
- const auto localeExists = [](const std::string name) {
- return bool(setlocale(LC_ALL, name.c_str()));
+ const auto fnSetLocale = [](const char *const name) {
+ return bool(setlocale(LC_NUMERIC, name));
};
- const char kSource[] = R"(
- void main()
+ const bool setLocaleToDe =
+ fnSetLocale("de_DE") || fnSetLocale("de-DE"); // Windows doesn't like de_DE.
+
+// These configs don't support de_DE: android_angle_vk[32,64]_rel_ng, linux_angle_rel_ng
+// Just allow those platforms to quietly fail, but require other platforms to succeed.
+#if defined(ANGLE_PLATFORM_ANDROID) || defined(ANGLE_PLATFORM_LINUX)
+ if (!setLocaleToDe)
{
- gl_FragColor = vec4(1.9);
- })";
- const char *parts[] = {kSource};
-
- for (const std::string &locale : availableLocales)
- {
- // If the locale doesn't exist on the testing platform, the locale constructor will fail,
- // throwing an exception
- // We use setlocale() (through localeExists) to test whether a locale
- // exists before calling the locale constructor
- if (localeExists(locale))
- {
- std::locale localizedLoc(locale);
-
- // std::locale::global() must be used instead of setlocale() to affect new streams'
- // default locale
- std::locale::global(std::locale::classic());
- sh::Compile(mCompiler, parts, 1, SH_OBJECT_CODE);
- std::string referenceOut = sh::GetObjectCode(mCompiler);
- EXPECT_NE(referenceOut.find("1.9"), std::string::npos)
- << "float formatted incorrectly with classic locale";
-
- sh::ClearResults(mCompiler);
-
- std::locale::global(localizedLoc);
- sh::Compile(mCompiler, parts, 1, SH_OBJECT_CODE);
- std::string localizedOut = sh::GetObjectCode(mCompiler);
- EXPECT_NE(localizedOut.find("1.9"), std::string::npos)
- << "float formatted incorrectly with locale (" << localizedLoc.name() << ") set";
-
- ASSERT_EQ(referenceOut, localizedOut)
- << "different output with locale (" << localizedLoc.name() << ") set";
- }
+ return;
}
-}
\ No newline at end of file
+#endif
+ ASSERT_TRUE(setLocaleToDe);
+
+ const char kSource[] = R"(
+ void main()
+ {
+ gl_FragColor = vec4(1.9);
+ })";
+ const char *parts[] = {kSource};
+ testCompile(parts, 1, true);
+
+ const auto &translated = sh::GetObjectCode(mCompiler);
+ // printf("%s\n", translated.data());
+ EXPECT_NE(translated.find("1.9"), std::string::npos);
+
+ fnSetLocale(defaultLocale);
+}