[1/3,gdb/tetsuite] Add missing include in gdb.base/ctf-ptype.c

  On fedora rawhide, when running test-case gdb.base/ctf-ptype.exp, I get:
...
gdb compile failed, ctf-ptype.c: In function 'main':
ctf-ptype.c:242:29: error: implicit declaration of function 'malloc' \
  [-Wimplicit-function-declaration]
  242 |   v_char_pointer = (char *) malloc (1);
      |                             ^~~~~~
ctf-ptype.c:1:1: note: include '<stdlib.h>' or provide a declaration of 'malloc'
  +++ |+#include <stdlib.h>
    1 | /* This test program is part of GDB, the GNU debugger.
...

Fix this by adding the missing include.

Tested on aarch64-linux.
---
 gdb/testsuite/gdb.base/ctf-ptype.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)


base-commit: 18d2988e5da8919514c76b83e2c0b56e439018bd
  

On 3/27/24 10:30 AM, Tom de Vries wrote:
> On fedora rawhide, when running test-case gdb.base/ctf-ptype.exp, I get:
> ...
> gdb compile failed, ctf-ptype.c: In function 'main':
> ctf-ptype.c:242:29: error: implicit declaration of function 'malloc' \
>    [-Wimplicit-function-declaration]
>    242 |   v_char_pointer = (char *) malloc (1);
>        |                             ^~~~~~
> ctf-ptype.c:1:1: note: include '<stdlib.h>' or provide a declaration of 'malloc'
>    +++ |+#include <stdlib.h>
>      1 | /* This test program is part of GDB, the GNU debugger.
> ...
> 
> Fix this by adding the missing include.
> 
> Tested on aarch64-linux.

Patches 2 and 3 look obvious to me (so Approved-By on this is fine by me)

> ---
>   gdb/testsuite/gdb.base/ctf-ptype.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/ctf-ptype.c b/gdb/testsuite/gdb.base/ctf-ptype.c
> index 4d2df33c53c..ca347893349 100644
> --- a/gdb/testsuite/gdb.base/ctf-ptype.c
> +++ b/gdb/testsuite/gdb.base/ctf-ptype.c
> @@ -24,6 +24,8 @@
>    *	First the basic C types.
>    */
>   
> +#include <stdlib.h>
> +
>   #if !defined (__STDC__) && !defined (_AIX)
>   #define signed  /**/
>   #endif
> @@ -234,9 +236,6 @@ func_type v_func_type;
>   
>   int main ()
>   {
> -  /* Ensure that malloc is a pointer type; avoid use of "void" and any include files. */
> -/*  extern char *malloc();*/
> -

This certainly seems curious.  Nothing in the commit log for when this was added
gives any clue to why this comment is here, nor why the prototype is commented
out.  Other comments later in this function mention limitations of the linker on
AIX, which makes me wonder how it was tested originally.  Did you confirm that
the test program compiled contained CTF but not DWARF when the test passed?
  

On 3/28/24 18:21, John Baldwin wrote:
> On 3/27/24 10:30 AM, Tom de Vries wrote:
>> On fedora rawhide, when running test-case gdb.base/ctf-ptype.exp, I get:
>> ...
>> gdb compile failed, ctf-ptype.c: In function 'main':
>> ctf-ptype.c:242:29: error: implicit declaration of function 'malloc' \
>>    [-Wimplicit-function-declaration]
>>    242 |   v_char_pointer = (char *) malloc (1);
>>        |                             ^~~~~~
>> ctf-ptype.c:1:1: note: include '<stdlib.h>' or provide a declaration 
>> of 'malloc'
>>    +++ |+#include <stdlib.h>
>>      1 | /* This test program is part of GDB, the GNU debugger.
>> ...
>>
>> Fix this by adding the missing include.
>>
>> Tested on aarch64-linux.
> 
> Patches 2 and 3 look obvious to me (so Approved-By on this is fine by me)
> 

Hi John,

thanks for the reviews.

I've committed these two.

>> ---
>>   gdb/testsuite/gdb.base/ctf-ptype.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/ctf-ptype.c 
>> b/gdb/testsuite/gdb.base/ctf-ptype.c
>> index 4d2df33c53c..ca347893349 100644
>> --- a/gdb/testsuite/gdb.base/ctf-ptype.c
>> +++ b/gdb/testsuite/gdb.base/ctf-ptype.c
>> @@ -24,6 +24,8 @@
>>    *    First the basic C types.
>>    */
>> +#include <stdlib.h>
>> +
>>   #if !defined (__STDC__) && !defined (_AIX)
>>   #define signed  /**/
>>   #endif
>> @@ -234,9 +236,6 @@ func_type v_func_type;
>>   int main ()
>>   {
>> -  /* Ensure that malloc is a pointer type; avoid use of "void" and 
>> any include files. */
>> -/*  extern char *malloc();*/
>> -
> 
> This certainly seems curious.  Nothing in the commit log for when this 
> was added
> gives any clue to why this comment is here, nor why the prototype is 
> commented
> out.  Other comments later in this function mention limitations of the 
> linker on
> AIX, which makes me wonder how it was tested originally.  Did you 
> confirm that
> the test program compiled contained CTF but not DWARF when the test passed?
> 

I did not, but I've done so now, I tested this on fedora asahi 39, 
aarch64-linux, and there's no dwarf in the exec, only ctf.

My guess of what happened during development of the test-case is:
- copy ptype.c to ctf-ptype.c
- run into some problem with including stdlib.h
- remove the include
- anticipate that compilation will fail due to malloc defaulting to
   int return type, and add comment and declaration
- either:
   - accidentally comment out declaration, or
   - intentionally comment out declaration, see that compilation actually
     works because a compiler-builtin declaration is used, rely on it and
     forget to update the comment to point that out

Avoiding void also doesn't make sense, given that the same commit that 
introduces the comment also introduces void.

Anyway, since it was commented out we've been using the gcc builtin 
prototype which has a void * return type.

So, I think reverting to using the include is the right thing to do.

It's possible the include was removed in early stages of support in the 
compiler or gdb where pulling in a header file pulled in unsupported stuff.

If there is indeed a problem, it'll pop up and we'll be able to fix it 
and add a proper comment explaining why we should avoid void or include 
files.

Thanks,
- Tom
  

>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:

>> int main ()
>> {
>> -  /* Ensure that malloc is a pointer type; avoid use of "void" and any include files. */
>> -/*  extern char *malloc();*/
>> -

John> This certainly seems curious.  Nothing in the commit log for when this was added
John> gives any clue to why this comment is here, nor why the prototype is commented
John> out.  Other comments later in this function mention limitations of the linker on
John> AIX, which makes me wonder how it was tested originally.  Did you confirm that
John> the test program compiled contained CTF but not DWARF when the test passed?

This file was apparently just copy-pasted from ptype.c, which has this
same commented-out line.  It probably dates to the stabs days.  I
wouldn't worry too much about it... it could use a scrubbing and perhaps
should be moved to gdb.ctf anyway.

Tom
  

On 4/1/24 17:49, Tom Tromey wrote:
>>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:
> 
>>> int main ()
>>> {
>>> -  /* Ensure that malloc is a pointer type; avoid use of "void" and any include files. */
>>> -/*  extern char *malloc();*/
>>> -
> 
> John> This certainly seems curious.  Nothing in the commit log for when this was added
> John> gives any clue to why this comment is here, nor why the prototype is commented
> John> out.  Other comments later in this function mention limitations of the linker on
> John> AIX, which makes me wonder how it was tested originally.  Did you confirm that
> John> the test program compiled contained CTF but not DWARF when the test passed?
> 
> This file was apparently just copy-pasted from ptype.c, which has this
> same commented-out line.  It probably dates to the stabs days.  I
> wouldn't worry too much about it... it could use a scrubbing and perhaps
> should be moved to gdb.ctf anyway.

OK, in that case ... pushed.

Thanks,
- Tom