2

I am using VTK together with MSVC and get a strange behaviour when trying to load data. I tinkered a little bit with it and even the following code generates a heap corruption, any ideas what is happening or what possibly went wrong?

vtkAbstractArray *vtkDataReader::ReadArray(const char *dataType, int numTuples, int numComp)
{

  char* type=strdup(dataType);

  free(type); // <--- here the heap corrution appears

  ...

This is the call stack:

>   msvcr90d.dll!_CrtIsValidHeapPointer(const void * pUserData=0x00691da0)  Zeile 2103  C++
    msvcr90d.dll!_free_dbg_nolock(void * pUserData=0x00691da0, int nBlockUse=1)  Zeile 1317 + 0x9 Bytes C++
    msvcr90d.dll!_free_dbg(void * pUserData=0x00691da0, int nBlockUse=1)  Zeile 1258 + 0xd Bytes    C++
    msvcr90d.dll!free(void * pUserData=0x00691da0)  Zeile 49 + 0xb Bytes    C++
    Simulator.exe!vtkDataReader::ReadArray(const char * dataType=0x0370b734, int numTuples=24576, int numComp=3)  Zeile 1401 + 0xc Bytes    C++
    Simulator.exe!vtkDataReader::ReadPoints(vtkPointSet * ps=0x081702d0, int numPts=24576)  Zeile 1936 + 0x15 Bytes C++

EDIT:

using this code instead of strdup works nicely, is strdup somehow broken on msvc?

char *type=(char*)malloc(100);
strcpy(type,dataType);
Dirk
  • 1,789
  • 2
  • 21
  • 31
  • 1
    Might need some more context here, what are you passing in as dataType? What operations are you doing between the `strdup` and `free` calls? – slugonamission Jan 05 '12 at 10:02
  • 2
    What are you doing between the `strdup` and the `free`? You probably write beyond the end of the duplicated string somewhere. – Some programmer dude Jan 05 '12 at 10:04
  • dataType is a char with the content "float" and there are no operations between strdup and free. – Dirk Jan 05 '12 at 10:05
  • That's strange then. I assume that `type` is a `char*`? – slugonamission Jan 05 '12 at 10:08
  • 3
    Is 'dataType' pointing to a _null_ terminated source string? If not, you are in trouble... – nabulke Jan 05 '12 at 10:10
  • @nabulke - I was just about to ask something similar, what does strlen(dataType) return? Are you sure you're actually passing a pointer to the first character and not a pointer to a pointer to the first character by accident? – slugonamission Jan 05 '12 at 10:12
  • 1
    Heap corrutptions are usually symptoms of invoking UB. One of the natures of UB is to not show as a crash (or not at all) exactly at the point where it is caused. As such, anything less than a minimal compiling test case will lead just to guessing around and eventually guessing the right thing. In any case, for collaborative SO debugging, the provded information is not enough. Start debugging yourself, think about what you needed to look at for doing this, and give us the same information. – PlasmaHH Jan 05 '12 at 10:12
  • @Dirk Could you post a _complete_ program that displays the problem? – Mr Lister Jan 05 '12 at 10:13
  • Try to check errno after strdup – Andrew Jan 05 '12 at 10:18
  • @nabulke this is dataType in memory: "66 6c 6f 61 74 00", strlen returns 5 – Dirk Jan 05 '12 at 10:20
  • Is the heap corruption still there if you remove the 'free' call? Just to make sure it is the troublemaker... – nabulke Jan 05 '12 at 10:30
  • @PlasmaHH you are probably right, i will try to create a minimal build... – Dirk Jan 05 '12 at 10:31
  • @nabulke it is gone if i remove free... – Dirk Jan 05 '12 at 10:35
  • it was indeed a problem with strdub... using _strdub instead fixes this, the answer below and upvote! – Dirk Jan 05 '12 at 11:02

1 Answers1

4

strdup as such is deprecated in msvc and there are reports of similar heap corruption around the web, microsoft states you should use _strdup instead

http://msdn.microsoft.com/en-us/library/ms235454

[EDIT: See below - the real cause seems to be both release and debug versions of the vs runtime dll being loaded, it's just a coincidence that _strdup fixes the problem]

Brian Arr
  • 131
  • 3
  • 1
    It is just the *name* that is deprecated. An implementation should not use names other than those specified in the language standard. Therefore `_strdup` is ok but `strdup` is not. – Bo Persson Jan 05 '12 at 10:57
  • 1
    you sir, are the hero of my day saving me hours of senseless debugging :) – Dirk Jan 05 '12 at 11:00
  • 1
    I will be _really_ surprised if using `__strdup` instead of `strdup` will fix that heap corruption. Dirk, can you confirm the corruption is gone? As Bo Persson stated, only the _name_ is deprecated, the function should definitely work just fine. – nabulke Jan 05 '12 at 11:07
  • I can confirm that using _strdup does fix the problem of msvc generating a heap corruption error. Nevertheless I cannot say if somewhere down the rabbit hole any undefined behavior is invoked... – Dirk Jan 05 '12 at 11:48
  • 1
    Doing some further research I found this: http://connect.microsoft.com/VisualStudio/feedback/details/333868/programs-using-a-static-library-and-calling-strdup-crash – Dirk Jan 05 '12 at 11:54
  • 1
    This link indicates that the root cause of the heap corruption is not using `strdup`, but linking to both the debug and retail/release versions of the microsoft runtime library in your application. So it seems you just got rid of a symptom and not the cause of this error. Maybe this could bite you again with another function? – nabulke Jan 05 '12 at 12:08
  • Isn't mixing debug and release code common pratice? Think of a library only available as a release build? – Dirk Jan 05 '12 at 12:16
  • No, I think mixing release and debug microsoft runtimes is not common practice and _can_ be dangerous. [Read more here...](http://stackoverflow.com/a/1227918/220636) – nabulke Jan 05 '12 at 12:23
  • but as long as I have a 3rd party library only as a release build I do not have any other option if I want do to do some debugging... don't I? – Dirk Jan 06 '12 at 12:27