Instance of user-defined data type ceases to exist - why?

[arnaudtemme]

Ladies and gentlemen,

In my model, I use grids:typedef double matelems;
typedef matelems *grid; and structs like struct tillage_parameters
{
grid *till_result;
int_grid *tillfields;
char fieldfile[30];
double tilc, plough_depth;
}; to store values for and pass values to modules that model a different process each. I practically have that running, thanks to bwkaz and others. Now, the different modules are in a time loop, so that for every year we can model changes to the landscape. The weird thing is that process "tillage", with its associated struct till_pars (see below) runs smoothly the first year, but then looses the grids from that struct before the second year. The error message is "access violation at etc.." , indicating that the grids from struct "till" that I use no longer exist or no longer occupy memory space. Below is the section within the timeloop that I found is responsible for the loss:
// before the timeloop: struct tillage_parameters till_pars;
// also before the timeloop: allocating memory space to grids in till_pars.

if (normal.use_tillage == 1) {
sprintf(ch, "1result 0 40 = %f\n", till_pars.till_result[0][40]);printf(ch);
sprintf(ch, "1field 0 40 = %d\n", till_pars.tillfields[0][40]);printf(ch);
till(&dtm, &ero_pars.soildepth, &till_pars,ascii,ero_pars);
add_double_grid(&dtm,till_pars.till_result,ascii.nr,ascii.nc);
sprintf(ch, "2result 0 40 = %f\n", till_pars.till_result[0][40]);printf(ch);
sprintf(ch, "2field 0 40 = %d\n", till_pars.tillfields[0][40]);printf(ch);
}

printf(till_pars.fieldfile);
sprintf(ch, "4result 0 40 = %f\n", till_pars.till_result[0][40]);printf(ch);
sprintf(ch, "4field 0 40 = %d\n", till_pars.tillfields[0][40]);printf(ch);

if (normal.use_water_erosion == 1) {
define_cellstatus(dtm, &dep.status_map, numberofsinks, ascii);
define_depressions(dtm, &dep.status_map, ascii, &dep);
define_fillheight(ascii, &dep);
erode(dtm, &waterflow, &ero_pars, &dep, ascii, evaporation, infiltration, rainfall);
add_double_grid(&dtm,ero_pars.erosion,ascii.nr,ascii.nc);
add_double_grid(&dtm,ero_pars.sedimentation,ascii.nr,ascii.nc);
}

write_asc_dbl(dtm,"dtm_4",ascii);
write_asc_dbl(ero_pars.erosion,"w_erosion",ascii);
write_asc_dbl(ero_pars.sedimentation,"w_sedimentation",ascii);
printf(" calculated water_erosion\n");
printf(till_pars.fieldfile);
sprintf(ch, "5result 0 40 = %f\n", till_pars.till_result[0][40]);printf(ch);
sprintf(ch, "5field 0 40 = %d\n", till_pars.tillfields[0][40]);printf(ch);
} // end run time loopWith output to screen:
calculated tectonics
1result 0 40 = 0
1field 0 40 = 1
2result 0 40 = 0.030841
2field 0 40 = 1
tilfield 4result 0 40 = 0.030841
4field 0 40 = 1
calculated water_erosion
tilfield And then the error message I mentioned above. My direct question is: how is it possible that the grids from till_pars loose the space they occupy in memory, whereas they are never mentioned in / passed to the other submodule for water erosion? And if it is a memory problem, why did it not occur before, because all the structs are initiated and all memory is allocated much earlier than this section of the code.

[truls]

I guess that ch is allocated on the stack, and if so it could be that you are writing too much to it and thus mess up the stack. The function will then crash when it tries to return to the point where is was called.

Try commenting out the last two lines and see if the program still crashes. Then comment out one bit at a time until you find out exactly which part is crashing.

[arnaudtemme]

I found that the problem occurs in define_depressions(dtm, &dep.status_map, ascii, &dep);which does not change the question. For your information: the use of both &dep.status_map and &dep in the call to define_depressions is not related to the problem, I changed that just now into sprintf(ch, "5bresult 0 40 = %f\n", till_pars.till_result[0][40]);printf(ch);
define_depressions(dtm, ascii, &dep);
sprintf(ch, "5cresult 0 40 = %f\n", till_pars.till_result[0][40]);printf(ch); and the problem persists. Output of the code above:
5bresult 0 40 = 0.030841 followed by error: "access violation at etc... ". So why can define_depressions change or destroy the grids in struct tillage_parameters till_pars? Note that till_pars is not a member of dem, ascii or dep...

[arnaudtemme]

Truls,

I understand what you are saying. I guess my update also functions as a reply to you, but just to make it sure : the first time after define_depressions that I try to read/write to any cell of either grid in struct till_pars, the crash occurs. Char-string 'ch' has a size that is more than ample: 100 chars.

[truls]

Well, since till_pars contains a multi-array, which is basically pointers to pointers, if define_depressions is faulty and overwrites any of these pointers you will get an access violation when trying to use it.

One way to check this could be to print out the pointer values of till_pars.till_result and see if they are the same when doing the successfull "4..." printout as right before you try to do the "5..." printout. Other than that I would need enough code to run it on my machine to do some debugging. Is it possible to reduce the code that crashes into a running program that can be debugged?

[bwkaz]

printf(till_pars.fieldfile); Umm... please, never, never do this. ;) If till_pars.fieldfile contains any percent characters, printf can walk off the stack into never-never land. (Depending on what's after the % characters.) The correct way to printf a string is to:

printf("%s", till_pars.fieldfile); This will never choke on the string.

(This is called a "format-string vulnerability"; you can search on that term and see all the colorful variations, if you really want to. But it's fairly simple to prevent: Never give printf anything other than a literal string for its first argument. If you have to print a string, use "%s".)

I doubt this is related to your crash, but who knows. If it doesn't help, then try running your program under gdb, and getting a backtrace. (Use "bt" to get gdb to print one.) Use "up" and "down" to move between stack frames, and use "print varname" to print the value of parameters, local variables, etc. Make sure you compile with -g though, otherwise gdb won't have any debug info to work with.

[arnaudtemme]

Thanks to both of you!

Truls, I have done that test you described:sprintf(ch, "5bresult 0 40 = %f\n", till_pars.till_result[0][40]);printf(ch);
printf("%d\n",till_pars.till_result);
define_depressions(dtm, ascii, &dep);
printf("%d\n",till_pars.till_result);
sprintf(ch, "5cresult 0 40 = %f\n", till_pars.till_result[0][40]);printf(ch); and the result was 5bresult 0 40 = 0.031873
14232608
0 meaning that you are probably right about define_depressions overwriting my dear till.pars grids. I am struggling with how to find the place where that happens in define_depressions. I will try to make a small version of the model and publish that tonight (Berlin time).


Bwkaz, okay I will no longer do that bad thing :). I tested it the right way and the problem persists, so indeed it was not the solution yet. I am unfamiliar with gdb, use borland 5.something. Normally I can backtrace mistakes quite okay with that, but in this case I would be looking for a method to somehow monitor the 14232608 memory space and see when it is occupied by anything from define_depressions. Please pardon the probably bad semantics here. You know of any way to do that in Borland or is that off limits ;)?

[arnaudtemme]

Guys,

here goes the code, I managed to downsize it to 67 kb. I guess putting it all in a project works for you as it does for me. It compiles and runs on my machine, of course until the error occurs. When running it, I noticed that now the error occurs on grid ero_pars.K_fac, but in a similar fashion: that grid exists with values before the call to define_depressions, but not afterwards. It is not used in define_depressions, even though ero_pars is passed to that function. So in this case the part of the code we discussed before looks like:sprintf(ch, "5bresult 0 40 = %f\n", ero_pars.K_fac[0][40]);printf(ch);
printf("%d\n",ero_pars.K_fac);
define_depressions(dtm, ascii, &dep);
printf("%d\n",ero_pars.K_fac);
sprintf(ch, "5cresult 0 40 = %f\n", ero_pars.K_fac[0][40]);printf(ch); with output 5bresult 0 40 = 0.004500
9984212
0 and then the error message. I hope you can find a way to pinpoint the error, even though function define_depressions is about .... 250 lines of code :D . Any hint to help me find the error myself is of course also highly appreciated. I guess that is the only option if you really need to understand what happens in the code before you can find the mistake.

Thanks a lot!

[truls]

Well, it compiles and runs fine on my mac. The major problem with the code is that the define_depression is not properly formatted, you have multiple statements on a single line, and it's too long. Break it up into smaller functions, format it properly, and you will stand a better chance at finding the bug.

It's probably not a good thing that I get a different output though...

[truls]

Im reformatting and reorganizing bits of the code to get an overview, but the first thing that struck me was this:
for(z=0;z<=numberofsinks;z++)
Are you sure that you want to use <=?
Arrays in C goes from 0 to size-1, so shouldn't you use < instead?

I'll try to work on this a bit in the weekend, but I have to say that you should really do something about the size and formatting of your functions. Divide and conquer is the name of the game here.

edit:
I've found at least two bugs in the code. There are two instances where you initiate a loop counter with == instead of =. Try compiling the sources with the -Wall option and fix all the warnings.

[arnaudtemme]

Truls,

thanks a lot for looking into this so deep. Indeed it is weird that the problem does not occur on your mac. Even weirder perhaps that the problem is solved on my machine too after I changed the initialization statement:Code:
for(z=0;z<numberofsinks;z++) though that really does not matter so much and fixed the (one!) bug in a for loop that I found. So I didn't find the second one .... I was expecting more trouble, this does not look like a mistake that overwrites my grids?

Regarding the huge amount of code, yes I agree I should divide and conquer. In fact I already have decided how I will subdivide define_depressions and sediment_in_depression from sinks.cpp into more than 10 other functions, but things have been too busy to get into that.

For your information: there are also some 'break' statements wrongly placed in the code. I repaired that too.

Thanks again

Arnaud

[truls]

Great that it's working. The reason it worked on my machine could probably be that the variables are laid out differently in memory. When you go one over on your machine, you set a pointer to zero. On my machine, it might be something different that is set to zero. But it would probably crash the program at one point.

There were two loops where you initiate the loop variable with == instead of =. But if you are compiling with g++, you can just add -Wall when compiling and it will tell you which lines the errors are on.