From 6d2b516f7406989f46f11828764a1cec44814a12 Mon Sep 17 00:00:00 2001 From: Benjamin Smedberg Date: Thu, 25 Jun 2009 15:12:19 -0400 Subject: [PATCH] Bug 491988 - do JS_REQUIRES_STACK analysis on function pointers, r=jorendorff+dmandelin --- js/src/analysis-tests/Makefile.in | 13 ++ js/src/analysis-tests/green-callredptr.cpp | 8 + js/src/analysis-tests/green-toredptr.cpp | 12 ++ .../misdeclared-red-funcptr.cpp | 5 + .../misdeclared-red-funcptr2.cpp | 5 + js/src/analysis-tests/red-togreenptr-arg.cpp | 11 + .../analysis-tests/red-togreenptr-field.cpp | 15 ++ .../red-togreenptr-initializer-array.cpp | 8 + .../red-togreenptr-initializer-cast.cpp | 7 + .../red-togreenptr-initializer-nested.cpp | 18 ++ .../red-togreenptr-initializer-struct.cpp | 12 ++ .../red-togreenptr-initializer.cpp | 7 + .../analysis-tests/red-togreenptr-return.cpp | 9 + js/src/analysis-tests/red-togreenptr.cpp | 10 + js/src/config/static-checking.js | 53 ++--- js/src/jsstack.js | 189 +++++++++++++++++- 16 files changed, 349 insertions(+), 33 deletions(-) create mode 100644 js/src/analysis-tests/green-callredptr.cpp create mode 100644 js/src/analysis-tests/green-toredptr.cpp create mode 100644 js/src/analysis-tests/misdeclared-red-funcptr.cpp create mode 100644 js/src/analysis-tests/misdeclared-red-funcptr2.cpp create mode 100644 js/src/analysis-tests/red-togreenptr-arg.cpp create mode 100644 js/src/analysis-tests/red-togreenptr-field.cpp create mode 100644 js/src/analysis-tests/red-togreenptr-initializer-array.cpp create mode 100644 js/src/analysis-tests/red-togreenptr-initializer-cast.cpp create mode 100644 js/src/analysis-tests/red-togreenptr-initializer-nested.cpp create mode 100644 js/src/analysis-tests/red-togreenptr-initializer-struct.cpp create mode 100644 js/src/analysis-tests/red-togreenptr-initializer.cpp create mode 100644 js/src/analysis-tests/red-togreenptr-return.cpp create mode 100644 js/src/analysis-tests/red-togreenptr.cpp diff --git a/js/src/analysis-tests/Makefile.in b/js/src/analysis-tests/Makefile.in index 4b3400471be..b180e331ecf 100644 --- a/js/src/analysis-tests/Makefile.in +++ b/js/src/analysis-tests/Makefile.in @@ -50,6 +50,18 @@ REDGREEN_FAILURE_TESTCASES = \ green-callred.cpp \ green-accessred.cpp \ green-tored-badpath.cpp \ + misdeclared-red-funcptr.cpp \ + misdeclared-red-funcptr2.cpp \ + red-togreenptr.cpp \ + red-togreenptr-return.cpp \ + red-togreenptr-arg.cpp \ + red-togreenptr-field.cpp \ + red-togreenptr-initializer.cpp \ + red-togreenptr-initializer-struct.cpp \ + red-togreenptr-initializer-array.cpp \ + red-togreenptr-initializer-nested.cpp \ + red-togreenptr-initializer-cast.cpp \ + green-callredptr.cpp \ $(NULL) REDGREEN_SUCCESS_TESTCASES = \ @@ -57,6 +69,7 @@ REDGREEN_SUCCESS_TESTCASES = \ red-callgreen.cpp \ red-accessred.cpp \ green-tored.cpp \ + green-toredptr.cpp \ $(NULL) STATIC_FAILURE_TESTCASES = \ diff --git a/js/src/analysis-tests/green-callredptr.cpp b/js/src/analysis-tests/green-callredptr.cpp new file mode 100644 index 00000000000..464e7f7e53d --- /dev/null +++ b/js/src/analysis-tests/green-callredptr.cpp @@ -0,0 +1,8 @@ +#include "jstypes.h" + +typedef void (JS_REQUIRES_STACK *RedFuncPtr)(); + +void GreenFunc(RedFuncPtr f) +{ + f(); +} diff --git a/js/src/analysis-tests/green-toredptr.cpp b/js/src/analysis-tests/green-toredptr.cpp new file mode 100644 index 00000000000..78df373208b --- /dev/null +++ b/js/src/analysis-tests/green-toredptr.cpp @@ -0,0 +1,12 @@ +#include "jstypes.h" + +void GreenFunc(); + +typedef void (JS_REQUIRES_STACK *RedFuncPtr)(); + +RedFuncPtr Test() +{ + // assigning a green function to a red function pointer is ok + RedFuncPtr p = GreenFunc; + return p; +} diff --git a/js/src/analysis-tests/misdeclared-red-funcptr.cpp b/js/src/analysis-tests/misdeclared-red-funcptr.cpp new file mode 100644 index 00000000000..98053476908 --- /dev/null +++ b/js/src/analysis-tests/misdeclared-red-funcptr.cpp @@ -0,0 +1,5 @@ +#include "jstypes.h" + + +// JS_REQUIRES_STACK should come before the * +typedef void (* JS_REQUIRES_STACK RedFuncPtr)(); diff --git a/js/src/analysis-tests/misdeclared-red-funcptr2.cpp b/js/src/analysis-tests/misdeclared-red-funcptr2.cpp new file mode 100644 index 00000000000..3ecc25e2b0b --- /dev/null +++ b/js/src/analysis-tests/misdeclared-red-funcptr2.cpp @@ -0,0 +1,5 @@ +#include "jstypes.h" + + +// JS_REQUIRES_STACK should come before the * +typedef void (* RedFuncPtr)() JS_REQUIRES_STACK; diff --git a/js/src/analysis-tests/red-togreenptr-arg.cpp b/js/src/analysis-tests/red-togreenptr-arg.cpp new file mode 100644 index 00000000000..b3bb17d74c5 --- /dev/null +++ b/js/src/analysis-tests/red-togreenptr-arg.cpp @@ -0,0 +1,11 @@ +#include "jstypes.h" + +typedef void (*GreenFuncPtr)(); +typedef void (JS_REQUIRES_STACK *RedFuncPtr)(); + +void TakesAsArgument(GreenFuncPtr g); + +void Test(RedFuncPtr p) +{ + TakesAsArgument(p); +} diff --git a/js/src/analysis-tests/red-togreenptr-field.cpp b/js/src/analysis-tests/red-togreenptr-field.cpp new file mode 100644 index 00000000000..f5c856b115e --- /dev/null +++ b/js/src/analysis-tests/red-togreenptr-field.cpp @@ -0,0 +1,15 @@ +#include "jstypes.h" + +typedef void (*GreenFuncPtr)(); +typedef void (JS_REQUIRES_STACK *RedFuncPtr)(); + +struct Foo +{ + int i; + GreenFuncPtr p; +}; + +void Test(Foo *foo, RedFuncPtr p) +{ + foo->p = p; +} diff --git a/js/src/analysis-tests/red-togreenptr-initializer-array.cpp b/js/src/analysis-tests/red-togreenptr-initializer-array.cpp new file mode 100644 index 00000000000..8e3ec186e7f --- /dev/null +++ b/js/src/analysis-tests/red-togreenptr-initializer-array.cpp @@ -0,0 +1,8 @@ +#include "jstypes.h" + +void GreenFunc(); +JS_REQUIRES_STACK void RedFunc(); + +typedef void (*GreenFuncPtr)(); + +GreenFuncPtr fpa[2] = {GreenFunc, RedFunc}; diff --git a/js/src/analysis-tests/red-togreenptr-initializer-cast.cpp b/js/src/analysis-tests/red-togreenptr-initializer-cast.cpp new file mode 100644 index 00000000000..8029605d650 --- /dev/null +++ b/js/src/analysis-tests/red-togreenptr-initializer-cast.cpp @@ -0,0 +1,7 @@ +#include "jstypes.h" + +JS_REQUIRES_STACK void RedFunc(); + +typedef void (*GreenFuncPtr)(int); + +GreenFuncPtr gfpa = (GreenFuncPtr) RedFunc; diff --git a/js/src/analysis-tests/red-togreenptr-initializer-nested.cpp b/js/src/analysis-tests/red-togreenptr-initializer-nested.cpp new file mode 100644 index 00000000000..d4e4815dcf5 --- /dev/null +++ b/js/src/analysis-tests/red-togreenptr-initializer-nested.cpp @@ -0,0 +1,18 @@ +#include "jstypes.h" + +void GreenFunc(); +JS_REQUIRES_STACK void RedFunc(); + +typedef void (*GreenFuncPtr)(); + +struct S +{ + int i; + GreenFuncPtr p; +}; + +S sa[] = { + { 2, GreenFunc }, + { 1, RedFunc }, +}; + diff --git a/js/src/analysis-tests/red-togreenptr-initializer-struct.cpp b/js/src/analysis-tests/red-togreenptr-initializer-struct.cpp new file mode 100644 index 00000000000..c2d9a9e35f5 --- /dev/null +++ b/js/src/analysis-tests/red-togreenptr-initializer-struct.cpp @@ -0,0 +1,12 @@ +#include "jstypes.h" + +JS_REQUIRES_STACK void RedFunc(); + +typedef void (*GreenFuncPtr)(); + +struct GreenStruct +{ + GreenFuncPtr func; +}; + +GreenStruct gs = { RedFunc }; diff --git a/js/src/analysis-tests/red-togreenptr-initializer.cpp b/js/src/analysis-tests/red-togreenptr-initializer.cpp new file mode 100644 index 00000000000..063bb14d3ac --- /dev/null +++ b/js/src/analysis-tests/red-togreenptr-initializer.cpp @@ -0,0 +1,7 @@ +#include "jstypes.h" + +JS_REQUIRES_STACK void RedFunc(); + +typedef void (*GreenFuncPtr)(); + +GreenFuncPtr funcp = RedFunc; diff --git a/js/src/analysis-tests/red-togreenptr-return.cpp b/js/src/analysis-tests/red-togreenptr-return.cpp new file mode 100644 index 00000000000..ff91afd4795 --- /dev/null +++ b/js/src/analysis-tests/red-togreenptr-return.cpp @@ -0,0 +1,9 @@ +#include "jstypes.h" + +typedef void (*GreenFuncPtr)(); +typedef void (JS_REQUIRES_STACK *RedFuncPtr)(); + +GreenFuncPtr Test(RedFuncPtr p) +{ + return p; +} diff --git a/js/src/analysis-tests/red-togreenptr.cpp b/js/src/analysis-tests/red-togreenptr.cpp new file mode 100644 index 00000000000..65dfd4d9d03 --- /dev/null +++ b/js/src/analysis-tests/red-togreenptr.cpp @@ -0,0 +1,10 @@ +#include "jstypes.h" + +JS_REQUIRES_STACK void RedFunc(); + +typedef void (*GreenFuncPtr)(); + +GreenFuncPtr Test() +{ + return RedFunc; +} diff --git a/js/src/config/static-checking.js b/js/src/config/static-checking.js index 6fba9f539c4..5159266df06 100644 --- a/js/src/config/static-checking.js +++ b/js/src/config/static-checking.js @@ -56,37 +56,28 @@ function hasAttribute(c, attrname) return false; } -function process_function(f, stmts) + +const forward_functions = [ + 'process_type', + 'process_tree_type', + 'process_decl', + 'process_tree_decl', + 'process_function', + 'process_tree', + 'process_cp_pre_genericize', + 'input_end' +]; + +function setup_forwarding(n) { - for each (let module in modules) - if (module.hasOwnProperty('process_function')) - module.process_function(f, stmts); + this[n] = function() { + for each (let module in modules) { + if (module.hasOwnProperty(n)) { + module[n].apply(this, arguments); + } + } + } } -function process_tree(fndecl) -{ - for each (let module in modules) - if (module.hasOwnProperty('process_tree')) - module.process_tree(fndecl); -} - -function process_decl(decl) -{ - for each (let module in modules) - if (module.hasOwnProperty('process_decl')) - module.process_decl(decl); -} - -function process_cp_pre_genericize(fndecl) -{ - for each (let module in modules) - if (module.hasOwnProperty('process_cp_pre_genericize')) - module.process_cp_pre_genericize(fndecl); -} - -function input_end() -{ - for each (let module in modules) - if (module.hasOwnProperty('input_end')) - module.input_end(); -} +for each (let n in forward_functions) + setup_forwarding(n); diff --git a/js/src/jsstack.js b/js/src/jsstack.js index b01de9a3a5e..18244957c2b 100644 --- a/js/src/jsstack.js +++ b/js/src/jsstack.js @@ -35,9 +35,10 @@ MapFactory.use_injective = true; */ const RED = 'JS_REQUIRES_STACK'; const TURN_RED = 'JS_FORCES_STACK'; +const IGNORE_ERRORS = 'JS_IGNORE_STACK'; function attrs(tree) { - let a = DECL_P(tree) ? DECL_ATTRIBUTES(tree) : TYPE_ATTRIBUTES(TREE_TYPE(tree)); + let a = DECL_P(tree) ? DECL_ATTRIBUTES(tree) : TYPE_ATTRIBUTES(tree); return translate_attributes(a); } @@ -53,6 +54,47 @@ function hasUserAttribute(tree, attrname) { return false; } +function process_tree_type(d) +{ + let t = dehydra_convert(d); + if (t.isFunction) + return; + + if (t.typedef !== undefined) + if (isRed(TYPE_NAME(d))) + error("Typedef declaration is annotated JS_REQUIRES_STACK: the annotation should be on the type itself", t.loc); + + if (hasAttribute(t, RED)) { + error("Non-function is annotated JS_REQUIRES_STACK", t.loc); + return; + } + + for (let st = t; st !== undefined && st.isPointer; st = st.type) { + if (hasAttribute(st, RED)) { + error("Non-function is annotated JS_REQUIRES_STACK", t.loc); + return; + } + + if (st.parameters) + return; + } +} + +function process_tree_decl(d) +{ + // For VAR_DECLs, walk the DECL_INITIAL looking for bad assignments + if (TREE_CODE(d) != VAR_DECL) + return; + + let i = DECL_INITIAL(d); + if (!i) + return; + + assignCheck(i, TREE_TYPE(d), function() { return location_of(d); }); + + functionPointerWalk(i, d); +} + /* * x is an expression or decl. These functions assume that */ @@ -61,6 +103,9 @@ function isTurnRed(x) { return hasUserAttribute(x, TURN_RED); } function process_tree(fndecl) { + if (hasUserAttribute(fndecl, IGNORE_ERRORS)) + return; + if (!(isRed(fndecl) || isTurnRed(fndecl))) { // Ordinarily a user of ESP runs the analysis, then generates output based // on the results. But in our case (a) we need sub-basic-block resolution, @@ -73,6 +118,8 @@ function process_tree(fndecl) if (a.hasRed) a.run(); } + + functionPointerCheck(fndecl); } function RedGreenCheck(fndecl, trace) { @@ -115,7 +162,7 @@ function RedGreenCheck(fndecl, trace) { } for (let i = stack.length - 1; i >= 0; --i) { - loc = location_of(stack[i]); + let loc = location_of(stack[i]); if (loc !== undefined) return loc; } @@ -145,6 +192,23 @@ function RedGreenCheck(fndecl, trace) { isn.turnRed = true; } } + else { + let fntype = TREE_CHECK( + TREE_TYPE( // the function type + TREE_TYPE( // the function pointer type + CALL_EXPR_FN(t) + ) + ), + FUNCTION_TYPE, METHOD_TYPE); + if (isRed(fntype)) { + isn.redInfo = ["cannot call JS_REQUIRES_STACK function pointer", + getLocation(false)]; + self.hasRed = true; + } + else if (isTurnRed(fntype)) { + isn.turnRed = true; + } + } } break; } @@ -186,3 +250,124 @@ RedGreenCheck.prototype.flowState = function(isn, state) { state.assignValue(this._state_var_decl, 1, isn); }; +function followTypedefs(type) +{ + while (type.typedef !== undefined) + type = type.typedef; + return type; +} + +function assignCheck(source, destType, locfunc) +{ + if (TREE_CODE(destType) != POINTER_TYPE) + return; + + let destCode = TREE_CODE(TREE_TYPE(destType)); + if (destCode != FUNCTION_TYPE && destCode != METHOD_TYPE) + return; + + if (isRed(TREE_TYPE(destType))) + return; + + while (TREE_CODE(source) == NOP_EXPR) + source = source.operands()[0]; + + // The destination is a green function pointer + + if (TREE_CODE(source) == ADDR_EXPR) { + let sourcefn = source.operands()[0]; + + // oddly enough, SpiderMonkey assigns the address of something that's not + // a function to a function pointer as part of the API! See JS_TN + if (TREE_CODE(sourcefn) != FUNCTION_DECL) + return; + + if (isRed(sourcefn)) + error("Assigning non-JS_REQUIRES_STACK function pointer from JS_REQUIRES_STACK function " + dehydra_convert(sourcefn).name, locfunc()); + } + else { + let sourceType = TREE_TYPE(TREE_TYPE(source).tree_check(POINTER_TYPE)); + switch (TREE_CODE(sourceType)) { + case FUNCTION_TYPE: + case METHOD_TYPE: + if (isRed(sourceType)) + error("Assigning non-JS_REQUIRES_STACK function pointer from JS_REQUIRES_STACK function pointer", locfunc()); + break; + } + } +} + +/** + * A type checker which verifies that a red function pointer is never converted + * to a green function pointer. + */ + +function functionPointerWalk(t, baseloc) +{ + walk_tree(t, function(t, stack) { + function getLocation(skiptop) { + if (!skiptop) { + let loc = location_of(t); + if (loc !== undefined) + return loc; + } + + for (let i = stack.length - 1; i >= 0; --i) { + let loc = location_of(stack[i]); + if (loc !== undefined) + return loc; + } + return location_of(baseloc); + } + + switch (TREE_CODE(t)) { + case GIMPLE_MODIFY_STMT: { + let [dest, source] = t.operands(); + assignCheck(source, TREE_TYPE(dest), getLocation); + break; + } + case CONSTRUCTOR: { + let ttype = TREE_TYPE(t); + switch (TREE_CODE(ttype)) { + case RECORD_TYPE: + case UNION_TYPE: { + for each (let ce in VEC_iterate(CONSTRUCTOR_ELTS(t))) + assignCheck(ce.value, TREE_TYPE(ce.index), getLocation); + break; + } + case ARRAY_TYPE: { + let eltype = TREE_TYPE(ttype); + for each (let ce in VEC_iterate(CONSTRUCTOR_ELTS(t))) + assignCheck(ce.value, eltype, getLocation); + break; + } + default: + warning("Unexpected type in initializer: " + TREE_CODE(TREE_TYPE(t)), getLocation()); + } + break; + } + case CALL_EXPR: { + // Check that the arguments to a function and the declared types + // of those arguments are compatible. + let ops = t.operands(); + let funcType = TREE_TYPE( // function type + TREE_TYPE(ops[1])); // function pointer type + let argTypes = [t for (t in function_type_args(funcType))]; + for (let i = argTypes.length - 1; i >= 0; --i) { + let destType = argTypes[i]; + let source = ops[i + 3]; + assignCheck(source, destType, getLocation); + } + break; + } + } + }); +} + +function functionPointerCheck(fndecl) +{ + let cfg = function_decl_cfg(fndecl); + for (let bb in cfg_bb_iterator(cfg)) + for (let isn in bb_isn_iterator(bb)) + functionPointerWalk(isn, DECL_SAVED_TREE(fndecl)); +}