Merge pull request #799 from kmod/format_checking

Enable validation of printf-style format strings

src/core/types.h

@@ -764,7 +764,8 @@ void addToSysArgv(const char* str);
 // The traceback given to the user will include this,
 // even though the execution didn't actually arrive there.
 void raiseSyntaxError(const char* msg, int lineno, int col_offset, llvm::StringRef file, llvm::StringRef func);
-void raiseSyntaxErrorHelper(llvm::StringRef file, llvm::StringRef func, AST* node_at, const char* msg, ...);
+void raiseSyntaxErrorHelper(llvm::StringRef file, llvm::StringRef func, AST* node_at, const char* msg, ...)
+    __attribute__((format(printf, 4, 5)));
 
 struct LineInfo {
 public:

src/runtime/builtin_modules/sys.cpp

@@ -88,8 +88,7 @@ BoxedList* getSysPath() {
     assert(_sys_path);
 
     if (_sys_path->cls != list_cls) {
-        fprintf(stderr, "RuntimeError: sys.path must be a list of directory name\n");
-        raiseExcHelper(RuntimeError, "");
+        raiseExcHelper(RuntimeError, "sys.path must be a list of directory names");
     }
 
     assert(_sys_path->cls == list_cls);

src/runtime/dict.cpp

@@ -604,14 +604,14 @@ void dictMergeFromSeq2(BoxedDict* self, Box* other) {
         if (element->cls == list_cls) {
             BoxedList* list = static_cast<BoxedList*>(element);
             if (list->size != 2)
-                raiseExcHelper(ValueError, "dictionary update sequence element #%d has length %d; 2 is required", idx,
+                raiseExcHelper(ValueError, "dictionary update sequence element #%d has length %ld; 2 is required", idx,
                                list->size);
 
             self->d[list->elts->elts[0]] = list->elts->elts[1];
         } else if (element->cls == tuple_cls) {
             BoxedTuple* tuple = static_cast<BoxedTuple*>(element);
             if (tuple->size() != 2)
-                raiseExcHelper(ValueError, "dictionary update sequence element #%d has length %d; 2 is required", idx,
+                raiseExcHelper(ValueError, "dictionary update sequence element #%d has length %ld; 2 is required", idx,
                                tuple->size());
 
             self->d[tuple->elts[0]] = tuple->elts[1];

src/runtime/file.cpp

@@ -433,8 +433,7 @@ static PyObject* get_line(BoxedFile* f, int n) noexcept {
 Box* fileRead(BoxedFile* self, Box* _size) {
     assert(self->cls == file_cls);
     if (_size->cls != int_cls) {
-        fprintf(stderr, "TypeError: an integer is required\n");
-        raiseExcHelper(TypeError, "");
+        raiseExcHelper(TypeError, "an integer is required");
     }
     int64_t size = static_cast<BoxedInt*>(_size)->n;
 
@@ -943,12 +942,10 @@ Box* fileNew(BoxedClass* cls, Box* s, Box* m, Box** args) {
         m = _PyUnicode_AsDefaultEncodedString(m, NULL);
 
     if (s->cls != str_cls) {
-        fprintf(stderr, "TypeError: coercing to Unicode: need string of buffer, %s found\n", getTypeName(s));
-        raiseExcHelper(TypeError, "");
+        raiseExcHelper(TypeError, "coercing to Unicode: need string of buffer, %s found", getTypeName(s));
     }
     if (m->cls != str_cls) {
-        fprintf(stderr, "TypeError: coercing to Unicode: need string of buffer, %s found\n", getTypeName(m));
-        raiseExcHelper(TypeError, "");
+        raiseExcHelper(TypeError, "coercing to Unicode: need string of buffer, %s found", getTypeName(m));
     }
 
     if (!PyInt_Check(buffering))
@@ -1111,7 +1108,7 @@ Box* fileIterNext(BoxedFile* s) {
     Box* rtn = fileReadline1(s);
     assert(!rtn || rtn->cls == str_cls);
     if (!rtn || ((BoxedString*)rtn)->s().empty())
-        raiseExcHelper(StopIteration, "");
+        raiseExcHelper(StopIteration, (const char*)NULL);
     return rtn;
 }
 

src/runtime/iterobject.cpp

@@ -103,7 +103,7 @@ Box* seqiterNext(Box* s) {
         else
             RELEASE_ASSERT(0, "");
         if (hasnext == False)
-            raiseExcHelper(StopIteration, "");
+            raiseExcHelper(StopIteration, (const char*)NULL);
     }
 
     RELEASE_ASSERT(self->next, "");

src/runtime/list.cpp

@@ -121,10 +121,9 @@ extern "C" Box* listPop(BoxedList* self, Box* idx) {
 
     if (n < 0 || n >= self->size) {
         if (self->size == 0)
-            fprintf(stderr, "IndexError: pop from empty list\n");
+            raiseExcHelper(IndexError, "pop from empty list");
         else
-            fprintf(stderr, "IndexError: pop index out of range\n");
-        raiseExcHelper(IndexError, "");
+            raiseExcHelper(IndexError, "pop index out of range");
     }
 
     Box* rtn = self->elts->elts[n];

src/runtime/long.cpp

@@ -668,9 +668,8 @@ BoxedLong* _longNew(Box* val, Box* _base) {
             Box* r = callattr(val, long_str, callattr_flags, NULL, NULL, NULL, NULL, NULL);
 
             if (!r) {
-                fprintf(stderr, "TypeError: long() argument must be a string or a number, not '%s'\n",
+                raiseExcHelper(TypeError, "TypeError: long() argument must be a string or a number, not '%s'\n",
                                getTypeName(val));
-                raiseExcHelper(TypeError, "");
             }
 
             if (isSubclass(r->cls, int_cls)) {

src/runtime/objmodel.cpp

@@ -256,7 +256,7 @@ extern "C" void assertFail(Box* assertion_type, Box* msg) {
         BoxedString* tostr = str(msg);
         raiseExcHelper(static_cast<BoxedClass*>(assertion_type), "%s", tostr->data());
     } else {
-        raiseExcHelper(static_cast<BoxedClass*>(assertion_type), "");
+        raiseExcHelper(static_cast<BoxedClass*>(assertion_type), (const char*)NULL);
     }
 }
 
@@ -1222,7 +1222,8 @@ Box* dataDescriptorInstanceSpecialCases(GetattrRewriteArgs* rewrite_args, BoxedS
 
                 Box* rtn = *reinterpret_cast<Box**>((char*)obj + member_desc->offset);
                 if (rtn == NULL) {
-                    raiseExcHelper(AttributeError, "%.*s", attr_name->size(), attr_name->data());
+                    assert(attr_name->data()[attr_name->size()] == '\0');
+                    raiseExcHelper(AttributeError, "%s", attr_name->data());
                 }
                 return rtn;
             }
@@ -1356,8 +1357,9 @@ Box* dataDescriptorInstanceSpecialCases(GetattrRewriteArgs* rewrite_args, BoxedS
         // is of that type.
 
         if (getset_descr->get == NULL) {
-            raiseExcHelper(AttributeError, "attribute '%.*s' of '%s' object is not readable", attr_name->size(),
-                           attr_name->data(), getTypeName(getset_descr));
+            assert(attr_name->data()[attr_name->size()] == '\0');
+            raiseExcHelper(AttributeError, "attribute '%s' of '%s' object is not readable", attr_name->data(),
+                           getTypeName(getset_descr));
         }
 
         // Abort because right now we can't call twice in a rewrite
@@ -2041,8 +2043,9 @@ bool dataDescriptorSetSpecialCases(Box* obj, Box* val, Box* descr, SetattrRewrit
 
         // TODO type checking goes here
         if (getset_descr->set == NULL) {
-            raiseExcHelper(AttributeError, "attribute '%.*s' of '%s' objects is not writable", attr_name->size(),
-                           attr_name->data(), getTypeName(obj));
+            assert(attr_name->data()[attr_name->size()] == '\0');
+            raiseExcHelper(AttributeError, "attribute '%s' of '%s' objects is not writable", attr_name->data(),
+                           getTypeName(obj));
         }
 
         if (rewrite_args) {
@@ -3235,7 +3238,7 @@ void rearrangeArguments(ParamReceiveSpec paramspec, const ParamNames* param_name
         Box* ovarargs = BoxedTuple::create(unused_positional.size(), &unused_positional[0]);
         getArg(varargs_idx, oarg1, oarg2, oarg3, oargs) = ovarargs;
     } else if (unused_positional.size()) {
-        raiseExcHelper(TypeError, "%s() takes at most %d argument%s (%d given)", func_name, paramspec.num_args,
+        raiseExcHelper(TypeError, "%s() takes at most %d argument%s (%ld given)", func_name, paramspec.num_args,
                        (paramspec.num_args == 1 ? "" : "s"), argspec.num_args + argspec.num_keywords + varargs.size());
     }
 
@@ -3335,7 +3338,7 @@ void rearrangeArguments(ParamReceiveSpec paramspec, const ParamNames* param_name
     for (int i = 0; i < paramspec.num_args - paramspec.num_defaults; i++) {
         if (params_filled[i])
             continue;
-        raiseExcHelper(TypeError, "%s() takes exactly %d arguments (%d given)", func_name, paramspec.num_args,
+        raiseExcHelper(TypeError, "%s() takes exactly %d arguments (%ld given)", func_name, paramspec.num_args,
                        argspec.num_args + argspec.num_keywords + varargs.size());
     }
 
@@ -4902,8 +4905,8 @@ extern "C" void delattrGeneric(Box* obj, BoxedString* attr, DelattrRewriteArgs*
     } else {
         // the exception cpthon throws is different when the class contains the attribute
         if (clsAttr != NULL) {
-            raiseExcHelper(AttributeError, "'%s' object attribute '%.*s' is read-only", getTypeName(obj), attr->size(),
-                           attr->data());
+            assert(attr->data()[attr->size()] == '\0');
+            raiseExcHelper(AttributeError, "'%s' object attribute '%s' is read-only", getTypeName(obj), attr->data());
         } else {
             assert(attr->data()[attr->size()] == '\0');
             raiseAttributeError(obj, attr->s());

src/runtime/objmodel.h

@@ -46,7 +46,8 @@ void default_free(void*);
 void dealloc_null(Box* box);
 
 // helper function for raising from the runtime:
-void raiseExcHelper(BoxedClass*, const char* fmt, ...) __attribute__((__noreturn__));
+void raiseExcHelper(BoxedClass*, const char* fmt, ...) __attribute__((__noreturn__))
+__attribute__((format(printf, 2, 3)));
 void raiseExcHelper(BoxedClass*, Box* arg) __attribute__((__noreturn__));
 
 BoxedModule* getCurrentModule();

src/runtime/types.cpp

@@ -1185,7 +1185,7 @@ static Box* typeCallInner(CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Bo
                 PyErr_SetString(TypeError, objectNewParameterTypeErrorMsg());
                 return NULL;
             } else
-                raiseExcHelper(TypeError, objectNewParameterTypeErrorMsg());
+                raiseExcHelper(TypeError, "%s", objectNewParameterTypeErrorMsg());
         }
     }