Application Crash and Faulting Offset

SGaist

@Juan-Dev said in Application Crash and Faulting Offset:

@mpergand no, application crashes when I do a specific action. Certain actions absolutely do not crash my application, but one in particular does. But the app doesn't always crash in exactly the same "place" when I perform this specific action. Basically, sometimes 100 lines of code are executed to crash and the next time only 50 lines.
@SGaist I'm looking at all my pointers. All must be initialized..? At least Null...?

All pointers that are not initialized at construction time shall be set to nullptr and all code that is using them should check whether they are non null before making use of them.

Juan Dev

@JonB
First of all thank you for all this information on pointers.
In my case the use of pointers is done with unsigned char* ptr to manage a block of memory.
A function is provided to me allowing me to allocate memory, I just have to pass it the address of a pointer and the desired memory size.
But for this pointer...

// I should declare it like this...?
unsigned char* ptr = null;
//or like this...?
unsigned char* ptr;
ptr = nullptr;

And you say "Allow it to crash" but how I can do that...?
Under QT Creator when I choose "Release Compilation" and launch my application with "CTRL+R" my application crashes at any given time. But when I choose to launch the application with "Start Debugging and F5", my application never crashes. But I could allow it to crash in Debug mode...?

Juan Dev

@SGaist Thank you for your explanations, I will look into all of this

Juan Dev

@mpergand I must to choose "Release Compilation" and after...?
Use the first button "Play" with CTRL+R
Or use the second button "Play" with F5 to start debugging...?

JonB

@Juan-Dev
You two examples are identical (use nullptr, I don't know if null even exists), doesn't matter which one you use. Pointers in themselves, whether nullptr or uninitialized, don't "crash". Only when you try to read from or write to them. Setting it to nullptr won't help if you dereference it before you set it to something suitable, though it might make a "crash" more likely.

What you report is "unusual". If a program compiled the same way crashes when run outside the debugger it would usually also crash from within the debugger, with the advantage that we get a stack trace. It's a different situation if you compile one for debug and the other way for release, but you don't seem to be doing that.

How do you actually know it has "crashed"? Does Windows show you a message? Getting much useful information out of just a crash from Event Viewer is hard and likely lead you nowhere.

If you really cannot get information about the crash, you are left with with putting judicious logs to file dotted around your code. After crash see what the last message was and try to narrow down from there.

Juan Dev

@JonB

From what I understand if I haven't properly initialized my "unsigned char" pointers and I attempt to write or read from those pointers I can potentially have crashes...?

And about "Release" and "Debug" :
I Have this "Setting"

If I launch my application with the first play button (or CTRL+R)

My application crashes (not at the beginning but when I do a specific action)
If I launch my Application with the second play button (or F5)

I get this message "This does not seem to be a Debug build". So I click on "OK"
And my Application never, ever, never crashes
But I have several messages of this type in the "Problems" window
Debugger encountered an exception: Exception at 0x7ffa14c5f39c, code: 0x71a: , flags=0x81 (first chance)
Debugger encountered an exception: Exception at 0x7ffa14c5f39c, code: 0xe06d7363: C++ exception, flags=0x81 (first chance)

And in the event viewer I always have this exception code: 0xc0000374 when my application crashes

JonB

@Juan-Dev said in Application Crash and Faulting Offset:

if I haven't properly initialized my "unsigned char" pointers and I attempt to write or read from those pointers I can potentially have crashes...?

Well of course! What else do you expect to happen if you read/write a random/0 area of memory? You must know this if you have written an app in the first place? Anyway by all means check all your pointers have a sensible value before you read from or write to where they point to.

Yes it is "odd" that your compiled code crashes when you run not under debugger but not when under debugger.

My application crashes (not at the beginning but when I do a specific action)

I suggest you post a screenshot or paste something of precisely what you see when it does "crash"? (Not what you come across in Event Viewer.) What are you seeing which even tells you your program has "crashed", you have not answered this?

SGaist

@Juan-Dev said in Application Crash and Faulting Offset:

In my case the use of pointers is done with unsigned char* ptr to manage a block of memory.
A function is provided to me allowing me to allocate memory, I just have to pass it the address of a pointer and the desired memory size.

@Juan-Dev, in addition to the @JonB's request, can you share the code that is related to that as well ? And if possible, tell us where that function comes from ?

Juan Dev

@JonB said in Application Crash and Faulting Offset:

I suggest you post a screenshot or paste something of precisely what you see when it does "crash"? (Not what you come across in Event Viewer.) What are you seeing which even tells you your program has "crashed", you have not answered this?

My application closes by itself. And I get the message (when I'm on QT) :
..\..\MSVC2019_64bit-Release\release\application_name.exe crashed.

SimonSchroeder

@Juan-Dev said in Application Crash and Faulting Offset:

// I should declare it like this...?
unsigned char* ptr = nullptr;

It is good practice to always initialize variables when you are declaring them. This can avoid a lot of problems. There are only very few cases where this is not (easily) possible. Sometimes it means declaring your variable a little later when all information is available. Bonus tip: mark as many variables as const as you can.

Juan Dev

First of all, thank you for all your response and your time spent.
In order to move forward as best as possible, I continued to search to find in the long code that this program composes, where the problem could arise.
I located the problem function and then I placed a return in different places in this function.

With the code below (an extract of the code in fact) my program runs correctly and the function exits correctly

unsigned char* contenuCrlFse = nullptr;
contenuCrlFse = (unsigned char*)malloc(SSV_LONG_CRL_FSE + 1 * sizeof(char));

/*
...
Here we find code that retrieves the content of contenuCrlFse
...
*/

qDebug() << "Before Clean Exit of the Function";
free(contenuCrlFse); return EXIT_SUCCESS;

// Finalization
contenuCrlFse[SSV_LONG_CRL_FSE] = '\0';

With the code below (an extract of the code in fact) the function exit does not take place

unsigned char* contenuCrlFse = nullptr;
contenuCrlFse = (unsigned char*)malloc(SSV_LONG_CRL_FSE + 1 * sizeof(char));

/*
...
Here we find code that retrieves the content of contenuCrlFse
...
*/

// Finalization
contenuCrlFse[SSV_LONG_CRL_FSE] = '\0';

qDebug() << "Before Clean Exit of the Function";
free(contenuCrlFse); return EXIT_SUCCESS;

And I had an error (in Debug mode) which appears and which is the following

And these two snippets use the constant below
#define SSV_LONG_CRL_FSE 40

JonB

@Juan-Dev
FWIW, with the code as shown, and nothing else (nothing in the commented out Here we find code that retrieves the content of contenuCrlFse section) should not generate the "buffer overrun" error. Have you tested it all on its own and in isolation from anything else? What else you might have from which this is an "extract" I cannot say..

You might output the hex value of contenuCrlFse pointer to compare against the address in the error message.

Juan Dev

With the help of your various comments I continued to look to find out where the problem came from and it certainly comes from the extraction. "code that retrieves the content".
I recreated a code snippet. Initially my code was as follows :
main.cpp

// Initialization
unsigned char* fullContent = nullptr;
size_t sizeFullContent = 0;
unsigned char* extractedContent = nullptr;
size_t sizeExtractedContent = 0;
int ret = 0;

// Get Full Content
ret = getFullContent(&fullContent,&sizeFullContent);  if(ret!=EXIT_SUCCESS) return EXIT_FAILURE;

// Memory Allocation
extractedContent = (unsigned char*)malloc(LONG_EXTRACT + 1 * sizeof(char));

// Content Extraction
ret = extractContent(fullContent,sizeFullContent,150,&extractedContent,&sizeExtractedContent);

// Extraction control
if(ret!=EXIT_SUCCESS) return EXIT_FAILURE;
if(sizeExtractedContent!=LONG_EXTRACT) return EXIT_FAILURE;

// Finalization
extractedContent[LONG_EXTRACT] = '\0';

functions.cpp

int extractContent(unsigned char* ptrMemoryFonc, size_t sizeMemoryFonc, int numFiled, unsigned char ** ptrExtractFonc, size_t * sizeExtractFonc)
{
  unsigned char* ptrExtract = nullptr;
  size_t sizeExtract = 0;
  size_t cursorStart = 0;

  // Here I have code to iterate through memory "ptrMemoryFonc" looking for "numField" and I get "cursorStart" and "sizeExtract"
  cursorStart = 21;
  sizeExtract = 10;

  // Memory Allocation
  ptrExtract = (unsigned char*)malloc(sizeExtract * sizeof(unsigned char));	if (ptrExtract == NULL) return EXIT_FAILURE;

  // For Each Character in the Field Found - Memorization
  for (size_t i = cursorStart; i < cursorStart + sizeExtract; i++)	ptrExtract[i - cursorStart] = ((unsigned char*)ptrMemoryFonc)[i];

  // Memorization
  *ptrExtractFonc = ptrExtract;
  *sizeExtractFonc = sizeExtract;

  return EXIT_SUCCESS;
}

In this initial code, I made the mistake of not freeing memory with free(extractedContent) in main.cpp
And when I wanted to free the memory that's when it generated an error.
So I reviewed my code and it is now the following :
main.cpp

// Initialization
unsigned char* fullContent = nullptr;
size_t sizeFullContent = 0;
unsigned char* extractedContent = nullptr;
size_t sizeExtractedContent = 0;
int ret = 0;

// Get Full Content
ret = getFullContent(&fullContent,&sizeFullContent);  if(ret!=EXIT_SUCCESS) return EXIT_FAILURE;

// Content Extraction
ret = extractContent(fullContent,sizeFullContent,150,&extractedContent,&sizeExtractedContent);

// Extraction control
if(ret!=EXIT_SUCCESS) return EXIT_FAILURE;
if(sizeExtractedContent!=LONG_EXTRACT) return EXIT_FAILURE;

// Finalization
extractedContent[LONG_EXTRACT] = '\0';

// Memory Release
free(extractedContent);

functions.cpp

int extractContent(unsigned char* ptrMemoryFonc, size_t sizeMemoryFonc, int numFiled, unsigned char ** ptrExtractFonc, size_t * sizeExtractFonc)
{
  unsigned char* ptrExtract = nullptr;
  size_t sizeExtract = 0;
  size_t cursorStart = 0;

  // Here I have code to iterate through memory "ptrMemoryFonc" looking for "numField" and I get "cursorStart" and "sizeExtract"
  cursorStart = 21;
  sizeExtract = 10;

  // Memory Allocation
  ptrExtract = (unsigned char*)malloc(sizeExtract+1 * sizeof(unsigned char));	if (ptrExtract == NULL) return EXIT_FAILURE;

  // For Each Character in the Field Found - Memorization
  for (size_t i = cursorStart; i < cursorStart + sizeExtract; i++)	ptrExtract[i - cursorStart] = ((unsigned char*)ptrMemoryFonc)[i];

  // Memorization
  *ptrExtractFonc = ptrExtract;
  *sizeExtractFonc = sizeExtract;

  return EXIT_SUCCESS;
}

What I changed :

In the "main.cpp" I no longer allocate memory with malloc for "extractedContent"
It is in the function that I allocate memory (adding 1 in anticipation of the '\0' character which will be added later)

But is the code correct...? Is my content extraction function correct...?

JonB

@Juan-Dev
Yes, it looks like in the old code you did not allocate room for the extra byte for ptrExtract. Now you do.

Since it is extractContent() which allocates room for the terminating \0 byte I would set that byte in extractContent() rather than in main.cpp for clarity, but that is up to you.

You code currently relies on the LONG_EXTRACT in main.cpp being equal to the sizeExtract calculated(?) in extractContent(). It could be less than that, but must not be more (because of the malloc()). This is the kind of "hidden" requirement which can be hard to spot if it goes wrong, you should link these two values as appropriate.

In case you are not aware, you might like to use std::memcpy( void* dest, const void* src, std::size_t count ) (or C memcpy()) to copy the bytes instead of your for loop:

std::memcpy(ptrExtract, ptrMemoryFonc + cursorStart, sizeExtract);

Less of your own code to check/clearer :)

Juan Dev

@JonB Thank for your answer

Since it is extractContent() which allocates room for the terminating \0 byte I would set that byte in extractContent() rather than in main.cpp for clarity, but that is up to you.

The extract function is sometimes used to extract an area to which I do not necessarily add a '\0' character. This is why I did not integrate this addition into the function itself.
But we agree that even if this character will not be added later, I can reserve a memory space with one more character in my function, this does not pose a problem...?

In case you are not aware, you might like to use std::memcpy() to copy the bytes instead of your for loop.
Less of your own code to check/clearer :)

We agree on the code to check :). Thank you for this valuable information. I'll look into setting this up.

JonB

@Juan-Dev said in Application Crash and Faulting Offset:

But we agree that even if this character will not be added later, I can reserve a memory space with one more character in my function, this does not pose a problem...?

Absolutely, this is fine. And you must do so in case you do add the terminator. So far as the malloc()/free() is concerned that works fine; what you do with/without the terminating \0 is a different matter.

You still have a "dependency" between the value of LONG_EXTRACT in the caller and the value of sizeExtract in extractContent()'s call to malloc(). If LONG_EXTRACT > sizeExtract you will have a "hidden" write of a byte beyond the malloc()ed area, which can be hard to spot. I would still make some relationship between those two values, even if it's just that the caller checks LONG_EXTRACT against the returned &sizeExtractedContent. If they are supposed to be the same extractedContent[sizeExtractedContent] = '\0'; would be safer. Using a separate LONG_EXTRACT here only makes sense if it can be less than sizeExtractedContent.

Juan Dev

@JonB
The part of the code I posted is confusing, I'm sorry.
But I don't always have a dependency in the full application code.

In the full application, the extractContent() function can be used to extract variable-length portions of memory.
And for some extracted portions I control the fixed length and add a '\0' character.
And for others I simply use the extracted portion without checking or adding characters