correctcomputation / checkedc-clang

This is the primary development repository for 3C, a tool for automatically converting legacy C code to the Checked C extension of C, which aims to enforce spatial memory safety. This repository is a fork of Checked C's.
14 stars 5 forks source link

Malloc not handled properly #96

Closed Machiry closed 4 years ago

Machiry commented 4 years ago

Consider the following code:

#define size_t int
extern _Itype_for_any(T) void *malloc(size_t size) : itype(_Array_ptr<T>) byte_count(size);

struct fptr { 
    int *values; 
    char *name;
};

void foo() {
    struct fptr * x = malloc(sizeof(struct fptr));    
    int *y = malloc(sizeof(int));
}

Gets converted to:

#define size_t int
extern _Itype_for_any(T) void *malloc(size_t size) : itype(_Array_ptr<T>) byte_count(size);

struct fptr { 
    _Ptr<int> values; 
    _Ptr<char> name;
};

void foo() {
    struct fptr * x = malloc(sizeof(struct fptr));    
    int *y = malloc(sizeof(int));
}

See that x and y are treated as WILD. They should be checked pointers.

Root cause

We check for sizeof expr using UnaryExprOrTypeTraitExpr, However, If you look at the AST of the argument, there is an implicit cast, which we do not handle:

ImplicitCastExpr 0x11e7d230 <col:34, col:52> 'int' <IntegralCast>
    |           `-UnaryExprOrTypeTraitExpr 0x11e7c710 <col:34, col:52> 'unsigned long' sizeof 'struct fptr':'struct fptr'

Fix

The fix for this is to normalize the expression before checking for UnaryExpr:

--- a/clang/lib/CConv/ConstraintResolver.cpp
+++ b/clang/lib/CConv/ConstraintResolver.cpp
@@ -125,7 +125,8 @@ static Atom *analyzeAllocExpr(Expr *E, Constraints &CS, QualType &ArgTy) {

   // Look for sizeof(X); return Arr or Ptr if found
   for (Expr *Ex: Exprs) {
-    UnaryExprOrTypeTraitExpr *arg = dyn_cast<UnaryExprOrTypeTraitExpr>(Ex);
+    UnaryExprOrTypeTraitExpr *arg =
+        dyn_cast<UnaryExprOrTypeTraitExpr>(getNormalizedExpr(Ex));

Note: This involves changing various tests.

mwhicks1 commented 4 years ago

Fixed