Skip to content

Fix remaining undefined behavior from ambiguous function returns #858

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lgritz merged 1 commit into from
Feb 14, 2018
Merged

Fix remaining undefined behavior from ambiguous function returns #858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 72 additions & 31 deletions src/liboslcomp/typecheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1187,10 +1187,14 @@ ASTfunction_call::typecheck_struct_constructor ()
/// If there is a tie (and only then), the return type is considered in the score.
/// If there is still not a single winner then a function is chosen by ranking
/// the possible return types, using the following precedence:
/// float, int, color, vector, point, normal, matrix, string
/// float, int, color, vector, point, normal, matrix, string, color closure,
/// struct, void
/// A warning is shown, printing which was function chosen and the list of all
/// that were considered ambiguous.
///
/// If two or more overloads differ only by return types that are both structs,
/// then the warning above is treated as an error.
///
/// Float to int coercion is scored, but is currently a synmonym for kNoMatch
/// as the spec does not allow implicit float to int conversion.
///
Expand Down Expand Up @@ -1426,6 +1430,8 @@ class CandidateFunctions {

std::pair<FunctionSymbol*, TypeSpec>
best(ASTNode* caller, const ustring& funcname) {
ASSERT (caller); // Assertion that passed ASTNode::ref was not empty

switch (m_candidates.size()) {
case 0:
// Nothing at all, Error
Expand Down Expand Up @@ -1460,7 +1466,45 @@ class CandidateFunctions {
ASSERT (c.first && c.first->sym);

if (ambiguity != -1) {
ASSERT (caller);

unsigned userstructs = 0;

auto rank = [&userstructs] (const TypeSpec& s) -> int {
// Arrays are currently not ranked as they cannot be returned.
ASSERT (!s.is_array());
ASSERT (!s.is_closure() || s.is_color_closure());

const TypeDesc& td = s.simpletype();
if (td == TypeDesc::TypeFloat)
return 0;
if (td == TypeDesc::TypeInt)
return 1;
if (td == TypeDesc::TypeColor)
return 2;
if (td == TypeDesc::TypeVector)
return 3;
if (td == TypeDesc::TypePoint)
return 4;
if (td == TypeDesc::TypeNormal)
return 5;
if (td == TypeDesc::TypeMatrix)
return 6;
if (td == TypeDesc::TypeString)
return 7;

if (s.is_color_closure())
return 8;
if (s.is_structure_based()) {
++userstructs;
return 9;
}
if (s.is_void())
return 10;

ASSERT (0 && "Unranked type");
return std::numeric_limits<int>::max();
};

if (true /*m_rval.simpletype().is_unknown()*/) {
// Ambiguity because the return type desired is unknown
// float noise(point p)
Expand All @@ -1473,43 +1517,40 @@ class CandidateFunctions {
// legible, and ambiguities will be reported in order they
// would be chosen.
std::sort(m_candidates.begin(), m_candidates.end(),
[](const Candidate& a, const Candidate& b) -> bool {
auto rank = [](const TypeSpec& s) -> int {
if (s == TypeDesc::TypeFloat)
return 0;
if (s == TypeDesc::TypeInt)
return 1;
if (s == TypeDesc::TypeColor)
return 2;
if (s == TypeDesc::TypeVector)
return 3;
if (s == TypeDesc::TypePoint)
return 4;
if (s == TypeDesc::TypeNormal)
return 5;
if (s == TypeDesc::TypeMatrix)
return 6;
if (s == TypeDesc::TypeString)
return 7;
return std::numeric_limits<int>::max();
};
return rank(a.rtype.simpletype())
< rank(b.rtype.simpletype());
[rank](const Candidate& a, const Candidate& b) -> bool {
return rank(a.rtype) < rank(b.rtype);
});

// New choice is now front of the list
c = std::make_pair(&m_candidates.front(),
m_candidates.front().rscore);
}

reportAmbiguity(caller, funcname, false, "Ambiguous call to");

candidateHeader("Chosen function is");
reportFunction(c.first->sym);
if (userstructs) {
// std::sort can call 'rank' multiple times, and we can't use an
// address to store the actual count (as we are sorting!).
userstructs = 0;
for (Candidates::const_reverse_iterator i = m_candidates.rbegin(),
e = m_candidates.rend(); i != e; ++i) {
if (i->rtype.is_structure_based() && ++userstructs > 1)
break;
}
}

candidateHeader("Other candidates are");
for (auto& candidate : m_candidates) {
if (candidate.sym != c.first->sym)
bool warn = userstructs < 2;
reportAmbiguity(caller, funcname, !warn /* "Candidates are" msg*/,
"Ambiguous call to", warn /* As warning */);
if (warn) {
candidateHeader("Chosen function is");
reportFunction(c.first->sym);

candidateHeader("Other candidates are");
for (auto& candidate : m_candidates) {
if (candidate.sym != c.first->sym)
reportFunction(candidate.sym);
}
} else {
for (auto& candidate : m_candidates)
reportFunction(candidate.sym);
}
}
Expand Down
132 changes: 90 additions & 42 deletions testsuite/function-overloads/ref/out.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,59 +1,103 @@
test.osl:177: warning: Ambiguous call to 'freturn ()'
test.osl:215: warning: Ambiguous call to 'freturn ()'
Chosen function is:
test.osl:71 float freturn ()
test.osl:73 float freturn ()
Other candidates are:
test.osl:68 int freturn ()
test.osl:72 color freturn ()
test.osl:73 vector freturn ()
test.osl:69 point freturn ()
test.osl:74 normal freturn ()
test.osl:75 matrix freturn ()
test.osl:70 string freturn ()
test.osl:178: warning: Ambiguous call to 'ireturn ()'
test.osl:70 int freturn ()
test.osl:74 color freturn ()
test.osl:76 vector freturn ()
test.osl:71 point freturn ()
test.osl:77 normal freturn ()
test.osl:78 matrix freturn ()
test.osl:72 string freturn ()
test.osl:75 closure color freturn ()
test.osl:80 struct A freturn ()
test.osl:79 void freturn ()
test.osl:216: warning: Ambiguous call to 'ireturn ()'
Chosen function is:
test.osl:80 int ireturn ()
test.osl:87 int ireturn ()
Other candidates are:
test.osl:77 color ireturn ()
test.osl:81 vector ireturn ()
test.osl:78 point ireturn ()
test.osl:82 normal ireturn ()
test.osl:83 matrix ireturn ()
test.osl:79 string ireturn ()
test.osl:179: warning: Ambiguous call to 'creturn ()'
test.osl:84 color ireturn ()
test.osl:88 vector ireturn ()
test.osl:85 point ireturn ()
test.osl:89 normal ireturn ()
test.osl:91 matrix ireturn ()
test.osl:86 string ireturn ()
test.osl:83 closure color ireturn ()
test.osl:82 struct A ireturn ()
test.osl:90 void ireturn ()
test.osl:217: warning: Ambiguous call to 'creturn ()'
Chosen function is:
test.osl:88 color creturn ()
test.osl:98 color creturn ()
Other candidates are:
test.osl:85 vector creturn ()
test.osl:87 point creturn ()
test.osl:86 normal creturn ()
test.osl:90 matrix creturn ()
test.osl:89 string creturn ()
test.osl:180: warning: Ambiguous call to 'vreturn ()'
test.osl:93 vector creturn ()
test.osl:97 point creturn ()
test.osl:94 normal creturn ()
test.osl:100 matrix creturn ()
test.osl:99 string creturn ()
test.osl:101 closure color creturn ()
test.osl:96 struct A creturn ()
test.osl:95 void creturn ()
test.osl:218: warning: Ambiguous call to 'vreturn ()'
Chosen function is:
test.osl:93 vector vreturn ()
test.osl:105 vector vreturn ()
Other candidates are:
test.osl:92 point vreturn ()
test.osl:95 normal vreturn ()
test.osl:94 matrix vreturn ()
test.osl:96 string vreturn ()
test.osl:181: warning: Ambiguous call to 'preturn ()'
test.osl:103 point vreturn ()
test.osl:109 normal vreturn ()
test.osl:106 matrix vreturn ()
test.osl:110 string vreturn ()
test.osl:104 closure color vreturn ()
test.osl:107 struct A vreturn ()
test.osl:108 void vreturn ()
test.osl:219: warning: Ambiguous call to 'preturn ()'
Chosen function is:
test.osl:101 point preturn ()
test.osl:118 point preturn ()
Other candidates are:
test.osl:99 normal preturn ()
test.osl:100 matrix preturn ()
test.osl:98 string preturn ()
test.osl:182: warning: Ambiguous call to 'nreturn ()'
test.osl:114 normal preturn ()
test.osl:116 matrix preturn ()
test.osl:112 string preturn ()
test.osl:115 closure color preturn ()
test.osl:117 struct A preturn ()
test.osl:113 void preturn ()
test.osl:220: warning: Ambiguous call to 'nreturn ()'
Chosen function is:
test.osl:104 normal nreturn ()
test.osl:122 normal nreturn ()
Other candidates are:
test.osl:103 matrix nreturn ()
test.osl:105 string nreturn ()
test.osl:183: warning: Ambiguous call to 'mreturn ()'
test.osl:120 matrix nreturn ()
test.osl:123 string nreturn ()
test.osl:124 closure color nreturn ()
test.osl:121 struct A nreturn ()
test.osl:125 void nreturn ()
test.osl:221: warning: Ambiguous call to 'mreturn ()'
Chosen function is:
test.osl:107 matrix mreturn ()
test.osl:130 matrix mreturn ()
Other candidates are:
test.osl:108 string mreturn ()
test.osl:131 string mreturn ()
test.osl:128 closure color mreturn ()
test.osl:129 struct A mreturn ()
test.osl:127 void mreturn ()
test.osl:222: warning: Ambiguous call to 'strreturn ()'
Chosen function is:
test.osl:136 string strreturn ()
Other candidates are:
test.osl:134 closure color strreturn ()
test.osl:133 struct A strreturn ()
test.osl:135 void strreturn ()
test.osl:223: warning: Ambiguous call to 'ccreturn ()'
Chosen function is:
test.osl:138 closure color ccreturn ()
Other candidates are:
test.osl:140 struct A ccreturn ()
test.osl:139 void ccreturn ()
test.osl:224: warning: Ambiguous call to 'structreturn ()'
Chosen function is:
test.osl:142 struct A structreturn ()
Other candidates are:
test.osl:143 void structreturn ()
test.osl:225: warning: Ambiguous call to 'structreturn1 ()'
Chosen function is:
test.osl:146 struct A structreturn1 ()
Other candidates are:
test.osl:145 void structreturn1 ()
Compiled test.osl -> test.oso
testA int
testB int
Expand Down Expand Up @@ -111,5 +155,9 @@ vreturn.vector
preturn.point
nreturn.normal
mreturn.matrix
strreturn.string
ccreturn.ccolor
structreturn.struct
structreturn1.struct


42 changes: 42 additions & 0 deletions testsuite/function-overloads/test.osl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,48 +65,86 @@ int funcb() { printf("funcb.int\n"); return 1; }
float funcb() { printf("funcb.float\n"); return 2; }
color funcb() { printf("funcb.color\n"); return 3; }

struct A { float a; };

int freturn() { printf("freturn.int\n"); return 1; }
point freturn() { printf("freturn.point\n"); return 1; }
string freturn() { printf("freturn.string\n"); return ""; }
float freturn() { printf("freturn.float\n"); return 1; }
color freturn() { printf("freturn.color\n"); return 1; }
closure color freturn() { printf("freturn.ccolor\n"); return diffuse(N); }
vector freturn() { printf("freturn.vector\n"); return 1; }
normal freturn() { printf("freturn.normal\n"); return 1; }
matrix freturn() { printf("freturn.matrix\n"); return 1; }
void freturn() { printf("freturn.void\n"); }
A freturn() { printf("freturn.struct\n"); A a; return a; }

A ireturn() { printf("ireturn.struct\n"); A a; return a; }
closure color ireturn() { printf("ireturn.ccolor\n"); return diffuse(N); }
color ireturn() { printf("ireturn.color\n"); return 1; }
point ireturn() { printf("ireturn.point\n"); return 1; }
string ireturn() { printf("ireturn.string\n"); return ""; }
int ireturn() { printf("ireturn.int\n"); return 1; }
vector ireturn() { printf("ireturn.vector\n"); return 1; }
normal ireturn() { printf("ireturn.normal\n"); return 1; }
void ireturn() { printf("ireturn.void\n"); }
matrix ireturn() { printf("ireturn.matrix\n"); return 1; }

vector creturn() { printf("creturn.vector\n"); return 1; }
normal creturn() { printf("creturn.normal\n"); return 1; }
void creturn() { printf("creturn.void\n"); }
A creturn() { printf("creturn.struct\n"); A a; return a; }
point creturn() { printf("creturn.point\n"); return 1; }
color creturn() { printf("creturn.color\n"); return 1; }
string creturn() { printf("creturn.string\n"); return ""; }
matrix creturn() { printf("creturn.matrix\n"); return 1; }
closure color creturn() { printf("creturn.ccolor\n"); return diffuse(N); }

point vreturn() { printf("vreturn.point\n"); return 1; }
closure color vreturn() { printf("vreturn.ccolor\n"); return diffuse(N); }
vector vreturn() { printf("vreturn.vector\n"); return 1; }
matrix vreturn() { printf("vreturn.matrix\n"); return 1; }
A vreturn() { printf("vreturn.struct\n"); A a; return a; }
void vreturn() { printf("vreturn.void\n"); }
normal vreturn() { printf("vreturn.normal\n"); return 1; }
string vreturn() { printf("vreturn.string\n"); return ""; }

string preturn() { printf("preturn.string\n"); return ""; }
void preturn() { printf("preturn.void\n"); }
normal preturn() { printf("preturn.normal\n"); return 1; }
closure color preturn() { printf("preturn.ccolor\n"); return diffuse(N); }
matrix preturn() { printf("preturn.matrix\n"); return 1; }
A preturn() { printf("preturn.struct\n"); A a; return a; }
point preturn() { printf("preturn.point\n"); return 1; }

matrix nreturn() { printf("nreturn.matrix\n"); return 1; }
A nreturn() { printf("nreturn.struct\n"); A a; return a; }
normal nreturn() { printf("nreturn.normal\n"); return 1; }
string nreturn() { printf("nreturn.string\n"); return ""; }
closure color nreturn() { printf("nreturn.ccolor\n"); return diffuse(N); }
void nreturn() { printf("nreturn.void\n"); }

void mreturn() { printf("mreturn.void\n"); }
closure color mreturn() { printf("mreturn.ccolor\n"); return diffuse(N); }
A mreturn() { printf("mreturn.struct\n"); A a; return a; }
matrix mreturn() { printf("mreturn.matrix\n"); return 1; }
string mreturn() { printf("mreturn.string\n"); return ""; }

A strreturn() { printf("strreturn.struct\n"); A a; return a; }
closure color strreturn() { printf("strreturn.ccolor\n"); return diffuse(N); }
void strreturn() { printf("strreturn.void\n"); }
string strreturn() { printf("strreturn.string\n"); return ""; }

closure color ccreturn() { printf("ccreturn.ccolor\n"); return diffuse(N); }
void ccreturn() { printf("ccreturn.void\n"); }
A ccreturn() { printf("ccreturn.struct\n"); A a; return a; }

A structreturn() { printf("structreturn.struct\n"); A a; return a; }
void structreturn() { printf("structreturn.void\n"); }

void structreturn1() { printf("structreturn1.void\n"); }
A structreturn1() { printf("structreturn1.struct\n"); A a; return a; }

shader test ()
{
{
Expand Down Expand Up @@ -181,6 +219,10 @@ shader test ()
preturn();
nreturn();
mreturn();
strreturn();
ccreturn();
structreturn();
structreturn1();
printf("\n");
}
}
Loading