2

I'm working on this problem for a while now and I can't figure out why it won't work. The code is just an extension of a working programm for solving a simulation problem. I've just added two string variables to the struct and also adjusted the neccessary functions, so that the additional parameters are handed over. Valgrind shows about 20 different conditional jump or move depends on uninitialised value(s) errors. All of these provide the same error message:

Uninitialised value was created by a stack allocation
at 0x401E23: addproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:360)

Below the respective functions:

typedef struct tproxel *pproxel;

typedef struct tproxel {
    int     id;                  /* unique proxel id for searching    */
    int     s;                   /* discrete state of SPN             */
    int     tau1k;               /* first supplementary variable      */
    int     tau2k;               /* second supplementary variable     */
    double  val;                 /* proxel probability                */
    string path;                 /* previous path                     */
    string output;               /* previous output                   */
    pproxel left, right;         /* pointers to child proxels in tree */
} proxel;


/* adds a new proxel to the tree */
void addproxel(int s, int tau1k, int tau2k, double val, string &path, string &output) {
    proxel *temp, *temp2;
    int cont = 1,id;

    /* Alarm! TAUMAX overstepped! */
    if (tau1k >= TAUMAX) {
        //  printf(">>> %3d %3d %3d %7.5le \n", s, tau1k, val, TAUMAX);
        tau1k = TAUMAX - 1;
    }

        /* compute id of new proxel */
    id = TAUMAX*(TAUMAX*s+tau1k)+tau2k;

    /* New tree, add root */
    if (root[sw] == NULL) {
        root[sw] = insertproxel(s,tau1k, tau2k, val, path, output);
        root[sw]->left = NULL;
        root[sw]->right = NULL;
        return;
    }

    /* Locate insertion point in tree */
    temp = root[sw];    
    while (cont == 1) {
        if ((temp->left != NULL) && (id < temp->id))
            temp = temp->left;
        else
            if ((temp->right != NULL) && (id > temp->id))
                temp = temp->right;
            else
                cont = 0;
    }

    /* Insert left leaf into tree */
    if ((temp->left == NULL) && (id < temp->id)) {
        temp2        = insertproxel(s, tau1k,tau2k, val, path, output);
        temp->left   = temp2;
        temp2->left  = NULL;
        temp2->right = NULL;
        return;
    }

    /* Insert right leaf into tree */
    if ((temp->right == NULL) && (id > temp->id)) {
        temp2        = insertproxel(s, tau1k,tau2k, val, path, output);
        temp->right  = temp2;
        temp2->left  = NULL;
        temp2->right = NULL;
        return;
    }

    /* Proxels have the same id, just add their vals */
    if (id == temp->id) {
        temp->val += val;
        return;
    }
    printf("\n\n\n!!!!!! addproxel failed !!!!!\n\n\n");
}

 /* compute size of tree */
int size(proxel *p) {
    int sl, sr;
    if (p == NULL)
        return(0);
    sl = size(p->left);
    sr = size(p->right);
    return(sl+sr+1);
}

Before the addition of the two string variables the code worked just fine, but now I'm getting only memory access violations. After hours of trying to get the code working, I have no idea, what could be wrong.

I hope someone can tell me what I'm missing, any help would be appreciated.

EDIT:

I've changed what was pointed out by @Jens back to the working code in the original programm and now I'm getting different errors:

==1900== 1 errors in context 1 of 4:
==1900== Invalid read of size 4
==1900==    at 0x5159218: std::string::assign(std::string const&) (in /usr/lib/libstdc++.so.6.0.13)
==1900==    by 0x401CF1: insertproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:347)
==1900==    by 0x401E48: addproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:374)
==1900==    by 0x402195: main (HNMM.cpp:465)
==1900==  Address 0xfffffffffffffff8 is not stack'd, malloc'd or (recently) free'd
==1900== 
==1900== 
==1900== 1 errors in context 2 of 4:
==1900== Use of uninitialised value of size 8
==1900==    at 0x5159218: std::string::assign(std::string const&) (in /usr/lib/libstdc++.so.6.0.13)
==1900==    by 0x401CF1: insertproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:347)
==1900==    by 0x401E48: addproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:374)
==1900==    by 0x402195: main (HNMM.cpp:465)
==1900==  Uninitialised value was created by a heap allocation
==1900==    at 0x4C274A8: malloc (vg_replace_malloc.c:236)
==1900==    by 0x401C57: insertproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:336)
==1900==    by 0x401E48: addproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:374)
==1900==    by 0x402195: main (HNMM.cpp:465)
==1900== 
==1900== 
==1900== 1 errors in context 3 of 4:
==1900== Conditional jump or move depends on uninitialised value(s)
==1900==    at 0x51591A5: std::string::assign(std::string const&) (in /usr/lib/libstdc++.so.6.0.13)
==1900==    by 0x401CF1: insertproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:347)
==1900==    by 0x401E48: addproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:374)
==1900==    by 0x402195: main (HNMM.cpp:465)
==1900==  Uninitialised value was created by a heap allocation
==1900==    at 0x4C274A8: malloc (vg_replace_malloc.c:236)
==1900==    by 0x401C57: insertproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:336)
==1900==    by 0x401E48: addproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:374)
==1900==    by 0x402195: main (HNMM.cpp:465)
==1900== 
==1900== 
==1900== 1 errors in context 4 of 4:
==1900== Conditional jump or move depends on uninitialised value(s)
==1900==    at 0x5159181: std::string::assign(std::string const&) (in /usr/lib/libstdc++.so.6.0.13)
==1900==    by 0x401CF1: insertproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:347)
==1900==    by 0x401E48: addproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:374)
==1900==    by 0x402195: main (HNMM.cpp:465)
==1900==  Uninitialised value was created by a heap allocation
==1900==    at 0x4C274A8: malloc (vg_replace_malloc.c:236)
==1900==    by 0x401C57: insertproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:336)
==1900==    by 0x401E48: addproxel(int, int, int, double, std::string&, std::string&) (HNMM.cpp:374)
==1900==    by 0x402195: main (HNMM.cpp:465)

Respective Code and the error producing Lines:

/* get a fresh proxel and copy data into it */
proxel *insertproxel(int s, int tau1k, int tau2k, double val, string &path, string &output) {
    proxel *temp;
    /* create new proxel or grab one from free list */
  if (firstfree == NULL)
    temp = (proxel*) malloc(sizeof(proxel));
  else {
    temp = firstfree;
    firstfree = firstfree->right;
  }
  /* copy values */
  temp->id    = TAUMAX*(TAUMAX*s+tau1k)+tau2k;
  temp->s     = s;
  temp->tau1k = tau1k;
  temp->tau2k = tau2k;
  temp->val   = val;
  temp->path  = string(path);
  temp->output  = string(output);
  ccpcnt     += 1;
  if (maxccp < ccpcnt) {
      maxccp = ccpcnt;
      //printf("\n ccpcnt=%d",ccpcnt);
  }
  return(temp);
}

line 336:

temp = (proxel*) malloc(sizeof(proxel));

line 347:

temp->path  = string(path);

line 374:

root[sw] = insertproxel(s,tau1k, tau2k, val, path, output);
Faoran
  • 23
  • 6
  • Which are the lines reported, both for the "conditional jump or move" and for the "value was created" messages? Also, it would be very helpful if you could reduce your code to a minimal, complete program that exhibits the problem. – Mike Seymour Aug 29 '12 at 10:04

1 Answers1

3

In insertproxel you are returning a reference to a stack allocated variable:

   proxel temp2 = {0};
   temp = &temp2;
   /* ... */
   return(temp);
Jens Kilian
  • 425
  • 2
  • 7
  • Thank you for your help and I've changed my code back to the original code for this function but now I'm getting more errors. You can find more details in my edited opening post. – Faoran Aug 29 '12 at 11:59
  • You can't use `malloc` to allocate a `struct tproxel` when that contains a `std::string`. The string constructor will not be called. Use `new tproxel`. (Oh, and the `typedef struct foo { ... } bar;` is a C idiom that you also don't need in C++ - the declaration of `struct foo` will make `foo` a type name.) – Jens Kilian Aug 29 '12 at 14:01
  • Once again thanks for your help, now everything is working as intended. – Faoran Aug 29 '12 at 15:32