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

[Tom de Vries <tdevries@suse.de>]

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