This patch fixes the following bugs identified by the Clang Static Analyzer:
- Dead stores
- A couple of memory leaks
- A buffer overflow vulnerability (possible in theory, but not in
practice)
- The wrong data type was being used as the argument to sizeof in a call
to malloc
- A use-after-free bug involving realloc
- Chris
diff --git a/compiler/AST/build.cpp b/compiler/AST/build.cpp
index 9c124d2..78fdba9 100644
--- a/compiler/AST/build.cpp
+++ b/compiler/AST/build.cpp
@@ -1701,21 +1701,27 @@ destructureTupleGroupedArgs(FnSymbol* fn, BlockStmt* tuple, Expr* base) {
}
FnSymbol* buildLambda(FnSymbol *fn) {
- static int nextId = 0;
+ static unsigned int nextId = 0;
char buffer[100];
- sprintf(buffer, "_chpl_lambda_%i", nextId++);
- char *name = (char *)malloc(strlen(buffer) + 1);
-
- strcpy(name, buffer);
+ /*
+ * The snprintf function is used to prevent a buffer overflow from occurring.
+ * Technically, an overflow can only occur if Chapel is compiled on a machine
+ * where an unsigned integer can represent numbers larger than 10^86, but it
+ * is better to guard against this behavior then leaving someone wondering
+ * why we didn't.
+ */
+ if (snprintf(buffer, 100, "_chpl_lambda_%i", nextId++) >= 100) {
+ INT_FATAL("Too many lambdas.");
+ }
if (!fn) {
- fn = new FnSymbol(astr(name));
- }
- else {
- fn->name = astr(name);
+ fn = new FnSymbol(astr(buffer));
+ } else {
+ fn->name = astr(buffer);
fn->cname = fn->name;
}
+
fn->addFlag(FLAG_COMPILER_NESTED_FUNCTION);
return fn;
}
diff --git a/compiler/AST/view.cpp b/compiler/AST/view.cpp
index 7461ab2..055801a 100644
--- a/compiler/AST/view.cpp
+++ b/compiler/AST/view.cpp
@@ -640,9 +640,6 @@ void html_view(const char* passName) {
INT_ASSERT(passName == currentPassName);
FILE* html_file;
const char* filename;
- const char* chpl_home = fdump_html_chpl_home;
- if (strlen(chpl_home) <= 0)
- chpl_home = CHPL_HOME;
fprintf(html_index_file, "<TR><TD>");
fprintf(html_index_file, "%s%s[%d]", passName,
diff --git a/compiler/backend/beautify.cpp b/compiler/backend/beautify.cpp
index dfebdff..af8c98c 100644
--- a/compiler/backend/beautify.cpp
+++ b/compiler/backend/beautify.cpp
@@ -231,11 +231,6 @@ void beautify(fileinfo* origfile) {
fprintf(outputfile, "%s", cp); /* output line */
break;
default:
- if (strncmp(cp, "case", 4) == 0)
- i = 1;
- else
- i = 0;
-
if ((indent == TRUE) && (cp[0] != '#'))
for (i = 0; i < 2*depth+justify; i++)
fprintf(outputfile, " ");
diff --git a/compiler/main/arg.cpp b/compiler/main/arg.cpp
index 3e6851e..576e128 100644
--- a/compiler/main/arg.cpp
+++ b/compiler/main/arg.cpp
@@ -260,7 +260,7 @@ process_args(ArgumentState *arg_state, int argc, char **aargv) {
} else {
arg_state->file_argument = (char **)realloc(
arg_state->file_argument,
- sizeof(char**) * (arg_state->nfile_arguments + 2));
+ sizeof(char*) * (arg_state->nfile_arguments + 2));
arg_state->file_argument[arg_state->nfile_arguments++] = *argv;
arg_state->file_argument[arg_state->nfile_arguments] = NULL;
}
diff --git a/compiler/main/version.cpp b/compiler/main/version.cpp
index 4e95bca..20e4412 100644
--- a/compiler/main/version.cpp
+++ b/compiler/main/version.cpp
@@ -7,6 +7,6 @@ void
get_version(char *v) {
v += sprintf(v, "%d.%s.%s", MAJOR_VERSION, MINOR_VERSION, UPDATE_VERSION);
if (BUILD_VERSION || developer)
- v += sprintf(v, ".%d", BUILD_VERSION);
+ sprintf(v, ".%d", BUILD_VERSION);
}
diff --git a/compiler/optimizations/removeEmptyRecords.cpp b/compiler/optimizations/removeEmptyRecords.cpp
index 1c10600..4b5c8d2 100644
--- a/compiler/optimizations/removeEmptyRecords.cpp
+++ b/compiler/optimizations/removeEmptyRecords.cpp
@@ -27,7 +27,6 @@ removeEmptyRecords() {
!ct->symbol->hasFlag(FLAG_EXTERN)) {
bool empty = true;
if (ct->symbol->hasFlag(FLAG_FIXED_STRING)) {
- empty = false;
break;
}
for_fields(field, ct) {
diff --git a/compiler/parser/processTokens.cpp b/compiler/parser/processTokens.cpp
index a0421eb..488a2e8 100644
--- a/compiler/parser/processTokens.cpp
+++ b/compiler/parser/processTokens.cpp
@@ -135,7 +135,6 @@ void processMultiLineComment() {
c = 0;
lastc = 0;
- lastlastc = 0;
depth = 1;
newString();
diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp
index b7889fe..e3c71d9 100644
--- a/compiler/resolution/functionResolution.cpp
+++ b/compiler/resolution/functionResolution.cpp
@@ -3963,7 +3963,6 @@ static std::string buildParentName(AList &arg_list, bool isFormal, Type *retType
*/
static AggregateType* createOrFindFunTypeFromAnnotation(AList &arg_list, CallExpr *call) {
AggregateType *parent;
- FnSymbol *parent_method;
SymExpr *retTail = toSymExpr(arg_list.tail);
Type *retType = retTail->var->type;
@@ -3971,13 +3970,11 @@ static AggregateType* createOrFindFunTypeFromAnnotation(AList &arg_list, CallExp
std::string parent_name = buildParentName(arg_list, false, retType);
if (functionTypeMap.find(parent_name) != functionTypeMap.end()) {
- std::pair<AggregateType*, FnSymbol*> ctfs = functionTypeMap[parent_name];
- parent = ctfs.first;
- parent_method = ctfs.second;
- }
- else {
- parent = createAndInsertFunParentClass(call, parent_name.c_str());
- parent_method = createAndInsertFunParentMethod(call, parent, arg_list, false, retType);
+ parent = functionTypeMap[parent_name].first;
+
+ } else {
+ parent = createAndInsertFunParentClass(call, parent_name.c_str());
+ FnSymbol* parent_method = createAndInsertFunParentMethod(call, parent, arg_list, false, retType);
functionTypeMap[parent_name] = std::pair<AggregateType*, FnSymbol*>(parent, parent_method);
}
diff --git a/compiler/resolution/generics.cpp b/compiler/resolution/generics.cpp
index 1144be4..06ddcb1 100644
--- a/compiler/resolution/generics.cpp
+++ b/compiler/resolution/generics.cpp
@@ -65,7 +65,7 @@ explainInstantiation(FnSymbol* fn) {
}
}
}
- len += sprintf(msg+len, ")");
+ sprintf(msg+len, ")");
if (callStack.n) {
USR_PRINT(callStack.v[callStack.n-1], msg);
} else {
diff --git a/compiler/util/files.cpp b/compiler/util/files.cpp
index 58863ea..ead9db0 100644
--- a/compiler/util/files.cpp
+++ b/compiler/util/files.cpp
@@ -374,6 +374,7 @@ static const char* mysystem_getresult(const char* command,
strcpy(result,"");
}
closefile(systemFile);
+ free(systemFile);
return astr(result); // canonicalize
}
@@ -807,30 +808,43 @@ static int sys_getcwd(char** path_out)
{
int sz = 128;
char* buf;
- char* got;
- int err = 0;
buf = (char*) malloc(sz);
if( !buf ) return ENOMEM;
+
while( 1 ) {
- got = getcwd(buf, sz);
- if( got != NULL ) break;
- else if( errno == ERANGE ) {
+ if ( getcwd(buf, sz) != NULL ) {
+ break;
+
+ } else if ( errno == ERANGE ) {
// keep looping but with bigger buffer.
- sz = 2*sz;
- got = (char*) realloc(buf, sz);
- if( ! got ) {
+ sz *= 2;
+
+ /*
+ * Realloc may return NULL, in which case we will need to free the memory
+ * initially pointed to by buf. This is why we store the result of the
+ * call in newP instead of directly into buf. If a non-NULL value is
+ * returned we update the buf pointer.
+ */
+ void* newP = realloc(buf, sz);
+
+ if (newP != NULL) {
+ buf = static_cast<char*>(newP);
+
+ } else {
free(buf);
return ENOMEM;
}
+
} else {
// Other error, stop.
- err = errno;
+ free(buf);
+ return errno;
}
}
*path_out = buf;
- return err;
+ return 0;
}
// Find the path to the running program
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/NeoTech
_______________________________________________
Chapel-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/chapel-developers
