LinuxQuestions.org
Welcome to the most active Linux Forum on the web.
Go Back   LinuxQuestions.org > Forums > Non-*NIX Forums > Programming
User Name
Password
Programming This forum is for all programming questions.
The question does not have to be directly related to Linux and any language is fair game.

Notices

Reply
 
Search this Thread
Old 07-26-2010, 06:38 AM   #1
Skaperen
Senior Member
 
Registered: May 2009
Location: WV, USA
Distribution: Slackware, CentOS, Ubuntu, Fedora, Timesys, Linux From Scratch
Posts: 1,777
Blog Entries: 20

Rep: Reputation: 115Reputation: 115
gcc 4.4.1 incorrectly detects free() of non-heap object


Here is the error message I'm getting:

Code:
error: attempt to free a non-heap object ‘this_split’
The code includes some code that defines a struct which includes a flag indicating whether the struct instance was allocated by malloc() or by the calling program in static or automatic storage. Calling programs thus have a choice where to create the object. When the struct is no longer needed, free() is called IF AND ONLY IF the flag indicating allocation is on. Yet, GCC fails to detect that (it would be an incredibly immense task to do that). But it goes ahead and claims there is an attempt to do this free when in fact there is not.

GCC keeps getting nastier and nastier with these kinds of tests. In the past they produce annoying warnings, cluttering up otherwise clean compiles. With so many nuisance warnings, genuine problems are missed. Now they have errors AND combine it with wrong code analysis.

When will the insanity end? Or when will there be a --noinsanecodechecks flag?
 
Old 07-26-2010, 07:21 AM   #2
johnsfine
Guru
 
Registered: Dec 2007
Distribution: Centos
Posts: 5,107

Rep: Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114
1) Do you have a full (compilable) sample of code so we can see how you get that error message?

2) In other situations where gcc sees a compile time error in the non reachable side of an if statement in an inlined function, I have used templating instead of a run time if to force the compiler to recognize earlier that the condition will be known at compile time so the unreachable code can be ignored at compile time.

That might be your best solution, except the usage you described doesn't seem to fit knowing the condition at compile time, which means the compiler shouldn't know at compile time that it is a non heap object, which means you shouldn't even have that error (thus the reason I asked to see actual code).
 
Old 07-26-2010, 09:39 AM   #3
Skaperen
Senior Member
 
Registered: May 2009
Location: WV, USA
Distribution: Slackware, CentOS, Ubuntu, Fedora, Timesys, Linux From Scratch
Posts: 1,777
Blog Entries: 20

Original Poster
Rep: Reputation: 115Reputation: 115
Yes, but it is huge. You can see the defining code here:

http://libh.slashusr.org/source/stri.../lib/h/split.h

Search for "split_end".

I don't know what you mean by templating.

There are macros involved. The structure is passed through to macros that generate the code with the conditional and the free() call. In one case the caller has defined a pointer. In another case the caller has defined the struct and is passing it with & prefixed to the struct name. Essentially what the compiler is seeing amounts to:

Code:
struct some_defined_struct this_one;
/*...*/
struct_init( & this_one );
/*...*/
struct_done( & this_one );
What is generated amounts to:

Code:
/* struct_init */
( & this_one )->was_allocated = 0;

/* struct_done */
if ( ( & this_one )->was_allocated ) {
    free( ( & this_one ) );
}
So it LOOKS like free is being passed a pointer to automatic (or perhaps in other cases, static ... but definitely not heap) storage. A human could see that it won't ever call free(). The compiler's analysis is not extensive enough to see it (it could omit generated code if it went that far, too). It would need to see enough to be sure the caller didn't goof up the API, either (for example, the caller could have mangled the struct and caused was_allocated to have a non-zero value ... then the error would be valid).

It looks like I may have to change the API design to accomodate this. But there will be a lot of existing code to change around because all calls to the macro that now include that conditional free() will have to either call a macro that does it, or call a different macro that does not, depending on the circumstances.

I suppose it may be generating all those free() calls in older compilers, even for code cases that never will call free(). I also wonder what it would do if I have a pointer that under some conditions is assigned a value from malloc() and in other conditions is assigned a pointer to an automatic or static area.
 
Old 07-26-2010, 01:25 PM   #4
johnsfine
Guru
 
Registered: Dec 2007
Distribution: Centos
Posts: 5,107

Rep: Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114
Quote:
Originally Posted by Skaperen View Post
I don't know what you mean by templating.
Sorry. I wasn't reading carefully. I thought you were talking about C++ code. Now I see this is C code.

Such problems are much harder to deal with in C.
 
Old 07-26-2010, 01:52 PM   #5
johnsfine
Guru
 
Registered: Dec 2007
Distribution: Centos
Posts: 5,107

Rep: Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114Reputation: 1114
I think you may be able to wrap the run time test of libh__sp->flags & SPLIT_OBJ_ALLOCATED as well as the free into a call to __builtin_choose_expr with its "constant" parameter defended by a call to __builtin_constant_p.

I think that would make the compiler avoid the compile time error in the cases where it has enough compile time knowledge to create the error.

So far as I can tell, you're posts don't quite sync up with the code you linked, so I can't give exact details about how you would use those builtin's and I also can't test your claim about when/why you get that error message.
 
Old 07-26-2010, 02:26 PM   #6
Skaperen
Senior Member
 
Registered: May 2009
Location: WV, USA
Distribution: Slackware, CentOS, Ubuntu, Fedora, Timesys, Linux From Scratch
Posts: 1,777
Blog Entries: 20

Original Poster
Rep: Reputation: 115Reputation: 115
The basic idea is a state in the struct, or somewhere, is used to decide if free() will be called or not. It never will be called unless the pointer came from malloc(). The compiler doesn't know if that is the case or not. It sees the expression of a call to free with a pointer, which in that compile unit, happens to only be for automatic storage. In that compile unit the test would never be true, so free would never be called. If the error message were "unreachable code", I'd be saying "duh".
 
Old 07-26-2010, 07:56 PM   #7
wje_lq
Member
 
Registered: Sep 2007
Location: Mariposa
Distribution: Debian lenny, Slackware 12
Posts: 809

Rep: Reputation: 178Reputation: 178
Quote:
Originally Posted by Skaperen View Post
GCC keeps getting nastier and nastier with these kinds of tests.
Somehow this doesn't sound right; gcc 4.3.3 doesn't have this problem; the following shell script
Code:
gcc --version; rm -f 1; cat <<EOD > 1.c; gcc -Werror -Wall 1.c -o 1; ./1

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/*--------------------------------------------------------------------------*/

typedef struct
{
  int  on_heap;
  char payload[80];

} asdftype;

/*--------------------------------------------------------------------------*/

void
kill_this(asdftype *arg_data)
{
  printf("%s\n",arg_data->payload);

  if(arg_data->on_heap)
  {
    free(arg_data);
  }

} /* kill_this() */

/*--------------------------------------------------------------------------*/

int
main(void)
{
  asdftype  stack_data;
  asdftype *heap_data;

  stack_data.on_heap=0;

  heap_data=malloc(sizeof(asdftype));

  if(heap_data==NULL)
  {
    fprintf(stderr,"malloc() failed\n");

    return 1;
  }

  heap_data->on_heap=1;

  strcpy((&stack_data)->payload,"this is stack data");
  strcpy(heap_data->payload,"this is heap data");

  kill_this(&stack_data);

  kill_this(heap_data);

  return 0;

} /* main() */
EOD
generates this on my system:
Code:
Copyright (C) 2008 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

this is stack data
this is heap data
What does it generate on your system?
 
Old 07-26-2010, 09:26 PM   #8
wje_lq
Member
 
Registered: Sep 2007
Location: Mariposa
Distribution: Debian lenny, Slackware 12
Posts: 809

Rep: Reputation: 178Reputation: 178
Oops. I forgot a line of output. Here's the output:
Code:
gcc (GCC) 4.3.3
Copyright (C) 2008 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

this is stack data
this is heap data
 
Old 07-26-2010, 09:32 PM   #9
dwhitney67
Senior Member
 
Registered: Jun 2006
Location: Maryland
Distribution: Kubuntu, Fedora, RHEL
Posts: 1,523

Rep: Reputation: 332Reputation: 332Reputation: 332Reputation: 332
Quote:
Originally Posted by Skaperen View Post
...

Code:
struct some_defined_struct this_one;
/*...*/
struct_init( & this_one );
/*...*/
struct_done( & this_one );
What is generated amounts to:

Code:
/* struct_init */
( & this_one )->was_allocated = 0;

/* struct_done */
if ( ( & this_one )->was_allocated ) {
    free( ( & this_one ) );
}
So it LOOKS like free is being passed a pointer to automatic (or perhaps in other cases, static ... but definitely not heap) storage. A human could see that it won't ever call free(). The compiler's analysis is not extensive enough to see it (it could omit generated code if it went that far, too). It would need to see enough to be sure the caller didn't goof up the API, either (for example, the caller could have mangled the struct and caused was_allocated to have a non-zero value ... then the error would be valid).
Your analysis only applies to what you expect to occur during runtime. In other words, yes, it is easy to see that the value for 'was_allocated' will be set to zero, before struct_done() is called, however from the compiler's perspective, it is merely substituting (expanding) your macros in place of the code and compiling it.

The compiler is not executing your application as it compiles the code, and thus all it sees is an object type, declared on the stack, and then a potential freeing of this object at a later time. The step where you set 'was_allocated' to 0 is a legal statement that the compiler is happy with this, but it does not retain this knowledge when it gets around to compiling the struct_done().

In conclusion, expect the compiler to produce a warning when it evaluates the line of code containing struct_done(). This warning will indicate that a non-heap object may potentially be freed.

---------------

@ wje_lq - Your code will produce the same warning it you optimize the code during compilation.
Code:
gcc -O3 -Werror -Wall 1.c -o 1
 
Old 07-27-2010, 12:13 AM   #10
wje_lq
Member
 
Registered: Sep 2007
Location: Mariposa
Distribution: Debian lenny, Slackware 12
Posts: 809

Rep: Reputation: 178Reputation: 178
Quote:
Originally Posted by dwhitney67 View Post
@ wje_lq - Your code will produce the same warning it you optimize the code during compilation.
Code:
gcc -O3 -Werror -Wall 1.c -o 1
Um, that's not what happened. The shell script, as modified:
Code:
gcc --version; rm -f 1; cat <<EOD > 1.c; gcc -O3 -Werror -Wall 1.c -o 1; ./1

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/*--------------------------------------------------------------------------*/

typedef struct
{
  int  on_heap;
  char payload[80];

} asdftype;

/*--------------------------------------------------------------------------*/

void
kill_this(asdftype *arg_data)
{
  printf("%s\n",arg_data->payload);

  if(arg_data->on_heap)
  {
    free(arg_data);
  }

} /* kill_this() */

/*--------------------------------------------------------------------------*/

int
main(void)
{
  asdftype  stack_data;
  asdftype *heap_data;

  stack_data.on_heap=0;

  heap_data=malloc(sizeof(asdftype));

  if(heap_data==NULL)
  {
    fprintf(stderr,"malloc() failed\n");

    return 1;
  }

  heap_data->on_heap=1;

  strcpy((&stack_data)->payload,"this is stack data");
  strcpy(heap_data->payload,"this is heap data");

  kill_this(&stack_data);

  kill_this(heap_data);

  return 0;

} /* main() */
EOD
The output:
Code:
gcc (GCC) 4.3.3
Copyright (C) 2008 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

this is stack data
this is heap data
 
Old 07-27-2010, 12:18 AM   #11
dwhitney67
Senior Member
 
Registered: Jun 2006
Location: Maryland
Distribution: Kubuntu, Fedora, RHEL
Posts: 1,523

Rep: Reputation: 332Reputation: 332Reputation: 332Reputation: 332
Quote:
Originally Posted by wje_lq View Post
Um, that's not what happened. The shell script, as modified:
Your right; I was compiling with 4.4.1. Even with 4.3.2 the warning does not appear (which makes sense).
 
Old 07-27-2010, 02:08 AM   #12
TimothyEBaldwin
Member
 
Registered: Mar 2009
Posts: 249

Rep: Reputation: 27
Either remove -Werror from the command line, so you only get warnings, or pass the pointer though inline assembler to cleanse it:

Code:
{
  sometype *temp;
  asm("" : "=g"(temp) : "0"(pointer));
  free(temp);
}
 
Old 08-09-2010, 10:51 AM   #13
Skaperen
Senior Member
 
Registered: May 2009
Location: WV, USA
Distribution: Slackware, CentOS, Ubuntu, Fedora, Timesys, Linux From Scratch
Posts: 1,777
Blog Entries: 20

Original Poster
Rep: Reputation: 115Reputation: 115
The slippery slope with this is that there will be too many warnings going on. The silly warnings need to be possible to disable. And since there is likely to be much disagreement on what is silly, that really needs to be possible everywhere.

I basically need a "I know what I am doing here and have vetted it carefully upon seeing your warning in previous compiles, so now STFU on this ONE issue" option for GCC.

Of the many warnings GCC gives, quite many are useful. I've fixed code in many cases as a result. But there needs to be some way to indicate that this part of the code really is correct even though no code changes have happened.

A common problem I have encountered is where GCC thinks a variable will POSSIBLY be uninitialized when used. It's limited analysis of the logic flow didn't or can't determine that no path that would fail to set that variable could ever happen. But this kind of warning is cheap to defeat by just arbitrarily initializing a variable even though that will always be a redundant one. But here we have a case where no solution like that can exist.

Well, maybe. I'm going to try a union when I get back to it. If I store values from malloc() into one union member, and store other values into the other union member, maybe it won't complain about this (although if it next complains about an uninitialized pointer being freed, it might be a mess again, because it's not cheap to arbitrarily initialize a pointer with a malloc call).
 
Old 09-16-2010, 06:20 AM   #14
snhillbr
LQ Newbie
 
Registered: Sep 2010
Posts: 2

Rep: Reputation: 0
Try This

Code:
#pragma GCC diagnostic ignored "-Wall"
freeing_function(pointer);  /* This is your offending statement, segregated into a function*/
Note, however that this will disable ALL warnings on the line.

Edit: Original post required gcc 4.5, in 4.4.1, we can only disable the warnings before the function is declared

Last edited by snhillbr; 09-16-2010 at 06:52 AM. Reason: Original post required gcc 4.5 not 4.4.1
 
  


Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is Off
HTML code is Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
ntfsresize detects device size incorrectly ternarybit Linux - Software 1 08-25-2009 11:50 AM
CPP: Multiple pointers to one heap object problem. RHLinuxGUY Programming 2 03-27-2008 09:01 AM
Linux (FC4) Incorrectly Detects UDMA Mode on Asus DVD E616P2 gillius Linux - Hardware 1 07-13-2005 09:35 PM
Disk Free space being incorrectly reported NiallC Linux - General 2 10-02-2004 05:06 PM
free bsd iso images burned incorrectly. xviddivxoggmp3 Red Hat 2 08-11-2004 11:07 PM


All times are GMT -5. The time now is 11:39 PM.

Main Menu
My LQ
Write for LQ
LinuxQuestions.org is looking for people interested in writing Editorials, Articles, Reviews, and more. If you'd like to contribute content, let us know.
Main Menu
Syndicate
RSS1  Latest Threads
RSS1  LQ News
Twitter: @linuxquestions
identi.ca: @linuxquestions
Facebook: linuxquestions Google+: linuxquestions
Open Source Consulting | Domain Registration